From 8d0fa8c9d25d5198dc73a4eeebde89840b73603d Mon Sep 17 00:00:00 2001 From: kristianwind Date: Wed, 16 Sep 2026 10:49:52 +0200 Subject: [PATCH] Build the download address from the plugin header, not from the forge The v1.4.0 release went out and the feed came back readable, with this in it: "browser_download_url": "http://57.129.89.117:25001/kw/woocommerce-email-discount-manager/..." That field is filled from the forge's own configured root URL, which is not necessarily the address the outside world reaches it on. A shop handed that would be downloading plugin code over plain HTTP from a raw IP on a non-standard port, bypassing the Cloudflare tunnel in front of gitea.nolimit.dk and breaking the day that address changes. parse_release() now rebuilds both the package and the release-page address from the repository address in the plugin header, which is the one place that already had to be right for any of this to work. tests/smoke.php pins it with the exact response the live feed returned. The release check had the same bug from the other end, and a worse version of it: it verified the address the API reported rather than the one the plugin builds, so it was a green check on a URL no shop ever uses. It now downloads the package the plugin would download and opens the zip to confirm it is the plugin and not an error page with a 200 on it. And it only runs on the forge named in "Update URI". The other one is a mirror; requiring a mirror to be anonymously readable failed the GitHub half of the v1.4.0 release for a repository that is private on purpose. ci/update-smoke.sh is the check none of the above can stand in for: install the previous version into a throwaway shop and let WordPress find, download and install the new one by itself. Every piece can be individually correct and the update still not arrive. Co-Authored-By: Claude Opus 5 (1M context) --- ci/publish-release.sh | 50 ++++++++++++++--- ci/update-smoke.sh | 106 +++++++++++++++++++++++++++++++++++++ includes/class-updater.php | 68 +++++++++++++++--------- tests/smoke.php | 38 +++++++++++++ 4 files changed, 231 insertions(+), 31 deletions(-) create mode 100755 ci/update-smoke.sh diff --git a/ci/publish-release.sh b/ci/publish-release.sh index ba26bf0..5ae3649 100755 --- a/ci/publish-release.sh +++ b/ci/publish-release.sh @@ -113,8 +113,25 @@ print(json.dumps({ fi ci_step "the feed, read the way a shop reads it" -# Deliberately no token on any request below. A "check" that sends credentials -# the shops do not have is the false positive that hides exactly this bug. + +# Only the forge named in "Update URI" actually serves shops. This script runs +# on both, and the other one is a mirror — requiring a mirror to be anonymously +# readable would fail every release for a repository that is private on purpose. +# (It did, on the first v1.4.0 run: GitHub is private and is not the feed.) +update_uri=$(sed -n -E 's/^[[:space:]]*\*[[:space:]]*Update URI:[[:space:]]*(.*)/\1/p' "$PLUGIN_MAIN" | head -1 | tr -d '[:space:]') +feed_host=$(printf '%s' "$update_uri" | sed -E 's|^[a-z]+://([^/]+).*|\1|') +this_host=$(printf '%s' "${GITHUB_SERVER_URL:-}" | sed -E 's|^[a-z]+://([^/]+).*|\1|') + +if [ -z "$update_uri" ]; then + echo "The plugin header has no Update URI, so there is no feed to check." + exit 0 +fi + +if [ "$feed_host" != "$this_host" ]; then + echo " $this_host is a mirror; shops read $feed_host. Nothing to verify here." + exit 0 +fi + feed=$(curl -fsS --max-time 30 "$release_api" 2>/dev/null || true) if [ -z "$feed" ]; then @@ -126,16 +143,35 @@ if [ -z "$feed" ]; then exit 1 fi -package=$(printf '%s' "$feed" | tr ',' '\n' | sed -n -E 's/.*"browser_download_url":[[:space:]]*"([^"]*\.zip)".*/\1/p' | head -1) -if [ -z "$package" ]; then +printf '%s' "$feed" | grep -q '"browser_download_url"' || { echo ci_fail "The release is readable, but it has no zip attached. WordPress would be offered an update it cannot download." exit 1 -fi +} -code=$(curl -s -o /dev/null -w '%{http_code}' -L --max-time 60 "$package") +# Checked at the address the PLUGIN builds, not the one the API reports. +# +# A forge fills browser_download_url from its own configured root URL, which is +# not necessarily the address the outside world reaches it on: gitea.nolimit.dk +# returns http://57.129.89.117:25001/... there, a plain-HTTP raw IP that +# bypasses the Cloudflare tunnel. Verifying that address would be verifying a +# URL no shop ever uses — a green check on the wrong thing, which is worse than +# no check. +package="$update_uri/releases/download/$TAG/$PLUGIN_SLUG.zip" +code=$(curl -s -o /tmp/wpci-feed.zip -w '%{http_code}' -L --max-time 120 "$package") echo " $package -> HTTP $code" -[ "$code" = 200 ] || { ci_fail "The package URL is not anonymously downloadable."; exit 1; } +[ "$code" = 200 ] || { ci_fail "The package URL the plugin will use is not anonymously downloadable."; exit 1; } + +# And it has to be the plugin, not an error page with a 200 on it. +python3 -c ' +import sys, zipfile +names = zipfile.ZipFile("/tmp/wpci-feed.zip").namelist() +main = sys.argv[1] + "/" + sys.argv[2] +if main not in names: + sys.exit("The downloaded zip does not contain " + main) +print(" it is the plugin: %d entries, %s present" % (len(names), main)) +' "$PLUGIN_SLUG" "$PLUGIN_MAIN" +rm -f /tmp/wpci-feed.zip echo echo "Published $TAG. A shop running this plugin will be offered it within twelve hours," diff --git a/ci/update-smoke.sh b/ci/update-smoke.sh new file mode 100755 index 0000000..3dce184 --- /dev/null +++ b/ci/update-smoke.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# Install the PREVIOUS version into a throwaway shop and let WordPress update +# it from the published feed, on its own. +# +# This is the only check that proves the whole chain: header, filter, feed, +# download address, zip layout and the folder WordPress unpacks into. Every +# part of it can be individually correct and the update still not arrive — +# a package URL on an address the shop cannot reach, a zip whose top-level +# directory is not the plugin slug, a version WordPress does not consider +# newer. Run it after a release. +# +# FROM_VERSION=1.3.1 ci/update-smoke.sh +set -euo pipefail + +cd "$(dirname "$0")/.." +. ci/lib.sh +ci_load_conf + +FROM_VERSION="${FROM_VERSION:-0.0.1}" +expected=$(ci_plugin_version) + +id="wpup-$$" +net="$id-net" +cleanup() { + docker rm -f "$id-db" "$id-cli" >/dev/null 2>&1 || true + docker volume rm -f "$id-wp" >/dev/null 2>&1 || true + docker network rm "$net" >/dev/null 2>&1 || true +} +trap cleanup EXIT + +echo "Installing $PLUGIN_SLUG $FROM_VERSION, then asking WordPress to update it. Expecting $expected." + +docker network create "$net" >/dev/null +docker volume create "$id-wp" >/dev/null +docker run -d --name "$id-db" --network "$net" \ + -e MARIADB_ROOT_PASSWORD=root -e MARIADB_DATABASE=wordpress \ + -e MARIADB_USER=wordpress -e MARIADB_PASSWORD=wordpress mariadb:lts >/dev/null +docker run --rm -v "$id-wp:/var/www/html" alpine chown 33:33 /var/www/html +docker run -d --name "$id-cli" --network "$net" -v "$id-wp:/var/www/html" \ + -e WORDPRESS_DB_HOST="$id-db" wordpress:cli sleep infinity >/dev/null +docker exec -u root "$id-cli" sh -c 'echo memory_limit=512M > /usr/local/etc/php/conf.d/zz-memory.ini' +wp() { docker exec "$id-cli" wp "$@"; } + +for i in $(seq 1 60); do + docker exec "$id-db" mariadb -uwordpress -pwordpress -e 'select 1' wordpress >/dev/null 2>&1 && break + [ "$i" = 60 ] && { echo "MariaDB never came up." >&2; exit 1; } + sleep 2 +done + +ci_step "a shop running the previous version" +wp core download --quiet +wp config create --dbname=wordpress --dbuser=wordpress --dbpass=wordpress --dbhost="$id-db" --quiet +wp core install --url=http://plugin.test --title="update smoke" --admin_user=admin \ + --admin_password=admin --admin_email=ci@example.com --skip-email --quiet +[ "$NEEDS_WOOCOMMERCE" = yes ] && wp plugin install woocommerce --activate --quiet + +# The working tree, with the version wound back. WordPress compares the feed +# against this header and nothing else, so this is what makes it an "older" +# install as far as the update check is concerned. +./ci/dist-files.sh | tr '\n' '\0' | tar --null -T - -cf - \ + | docker exec -i "$id-cli" sh -c \ + "mkdir -p /var/www/html/wp-content/plugins/$PLUGIN_SLUG \ + && tar -xf - -C /var/www/html/wp-content/plugins/$PLUGIN_SLUG" +docker exec "$id-cli" sed -i -E \ + "s/^([[:space:]]*\*[[:space:]]*Version:[[:space:]]*).*/\1$FROM_VERSION/" \ + "/var/www/html/wp-content/plugins/$PLUGIN_SLUG/$PLUGIN_MAIN" +wp plugin activate "$PLUGIN_SLUG" --quiet +installed=$(wp plugin get "$PLUGIN_SLUG" --field=version) +echo " installed $installed" +[ "$installed" = "$FROM_VERSION" ] || { echo "Winding the version back did not take." >&2; exit 1; } + +ci_step "what WordPress thinks, unprompted" +# Not the plugin's own code — WordPress's update check, the same one that runs +# on a schedule in a live shop. +wp plugin update --all --dry-run 2>&1 | sed 's/^/ /' || true +available=$(wp plugin list --name="$PLUGIN_SLUG" --field=update) +echo " update status: $available" +[ "$available" = available ] || { + echo "WordPress was not offered an update. The header, the filter or the feed is wrong." >&2 + exit 1 +} + +ci_step "letting WordPress install it" +wp plugin update "$PLUGIN_SLUG" 2>&1 | sed 's/^/ /' + +now=$(wp plugin get "$PLUGIN_SLUG" --field=version) +status=$(wp plugin list --name="$PLUGIN_SLUG" --field=status) +echo +echo " version after update: $now (was $FROM_VERSION)" +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') +docker exec "$id-cli" ls /var/www/html/wp-content/plugins | sed 's/^/ /' + +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." + +echo +echo "WordPress found, downloaded and installed $expected from the published feed on its own." diff --git a/includes/class-updater.php b/includes/class-updater.php index fb1ec6a..1ad0d95 100644 --- a/includes/class-updater.php +++ b/includes/class-updater.php @@ -176,49 +176,69 @@ class WC_EDM_Updater { return null; } - $body = json_decode( wp_remote_retrieve_body( $response ), true ); + $release = $this->parse_release( json_decode( wp_remote_retrieve_body( $response ), true ) ); - if ( ! is_array( $body ) || empty( $body['tag_name'] ) ) { + if ( ! $release ) { set_transient( self::TRANSIENT, 'none', 15 * MINUTE_IN_SECONDS ); return null; } - $package = ''; - $tested = ''; + set_transient( self::TRANSIENT, $release, self::TTL ); + + return $release; + } + + /** + * Turn a release API response into what WordPress needs. + * + * The download address is rebuilt from the repository address in the + * plugin header rather than taken from the response's + * browser_download_url. That field is built from the forge's own + * configured root URL, which is not necessarily the address the outside + * world reaches it on — gitea.nolimit.dk returns + * "http://57.129.89.117:25001/..." there, a plain-HTTP raw IP that + * bypasses the Cloudflare tunnel in front of it. A shop handed that would + * be downloading plugin code over an unencrypted connection to an address + * that is free to change. Measured on the v1.4.0 release. + * + * @param mixed $body Decoded API response. + * @return array|null { version, package, url, tested }, or null. + */ + private function parse_release( $body ) { + if ( ! is_array( $body ) || empty( $body['tag_name'] ) ) { + return null; + } + + $tag = $body['tag_name']; + $base = rtrim( self::update_uri(), '/' ); + $zip = ''; foreach ( isset( $body['assets'] ) ? $body['assets'] : array() as $asset ) { - if ( empty( $asset['name'] ) || empty( $asset['browser_download_url'] ) ) { - continue; - } - if ( substr( $asset['name'], -4 ) === '.zip' ) { - $package = $asset['browser_download_url']; + if ( ! empty( $asset['name'] ) && '.zip' === substr( $asset['name'], -4 ) ) { + $zip = $asset['name']; } } + if ( '' === $zip ) { + // A release with no zip attached is a half-finished release. + // Offering it would hand WordPress a download it cannot install. + return null; + } + // The release notes carry the compatibility line the release workflow // writes, so wp-admin can show what this version was actually tested // against instead of what the previous one claimed. + $tested = ''; if ( ! empty( $body['body'] ) && preg_match( '/^Tested up to:\s*(\S+)/mi', $body['body'], $m ) ) { $tested = $m[1]; } - if ( '' === $package ) { - // A release with no zip attached is a half-finished release. - // Offering it would hand WordPress a download it cannot install. - set_transient( self::TRANSIENT, 'none', 15 * MINUTE_IN_SECONDS ); - return null; - } - - $release = array( - 'version' => ltrim( $body['tag_name'], 'vV' ), - 'package' => $package, - 'url' => ! empty( $body['html_url'] ) ? $body['html_url'] : $uri, + return array( + 'version' => ltrim( $tag, 'vV' ), + 'package' => $base . '/releases/download/' . rawurlencode( $tag ) . '/' . rawurlencode( $zip ), + 'url' => $base . '/releases/tag/' . rawurlencode( $tag ), 'tested' => $tested, ); - - set_transient( self::TRANSIENT, $release, self::TTL ); - - return $release; } /** diff --git a/tests/smoke.php b/tests/smoke.php index dc858aa..28673f8 100644 --- a/tests/smoke.php +++ b/tests/smoke.php @@ -233,6 +233,44 @@ wc_edm_is( $derive->invoke( new WC_EDM_Updater(), 'https://gitea.nolimit.dk/kw/demo' ) ); +// The forge builds browser_download_url from its own configured root URL, +// which is not necessarily the address the outside world reaches it on. This +// is the real response gitea.nolimit.dk returned for v1.4.0: a plain-HTTP raw +// IP that bypasses the Cloudflare tunnel in front of it. A shop handed that +// would be fetching plugin code over an unencrypted connection to an address +// that is free to change, so the download address is rebuilt from the +// repository address in the plugin header instead. +$parse = new ReflectionMethod( 'WC_EDM_Updater', 'parse_release' ); +$parse->setAccessible( true ); +$parsed = $parse->invoke( + new WC_EDM_Updater(), + array( + 'tag_name' => 'v1.4.0', + 'body' => "notes\n\nTested up to: 7.1\nWC tested up to: 11.1.0\n", + 'html_url' => 'http://57.129.89.117:25001/kw/woocommerce-email-discount-manager/releases/tag/v1.4.0', + 'assets' => array( + array( + 'name' => 'woocommerce-email-discount-manager.zip', + 'browser_download_url' => 'http://57.129.89.117:25001/kw/woocommerce-email-discount-manager/releases/download/v1.4.0/woocommerce-email-discount-manager.zip', + ), + ), + ) +); + +wc_edm_is( 'the version comes from the tag, without the v', '1.4.0', $parsed['version'] ); +wc_edm_is( + 'the download address is rebuilt on the host in the plugin header', + $update_uri . '/releases/download/v1.4.0/woocommerce-email-discount-manager.zip', + $parsed['package'] +); +wc_edm_ok( + 'so it is never the raw address the forge happened to report', + false === strpos( $parsed['package'], '57.129.89.117' ) && 0 === strpos( $parsed['package'], 'https://' ), + 'got ' . $parsed['package'] +); +wc_edm_is( 'and the tested-up-to line is read out of the release notes', '7.1', $parsed['tested'] ); +wc_edm_ok( 'a release with no zip attached is not offered at all', null === $parse->invoke( new WC_EDM_Updater(), array( 'tag_name' => 'v9.9.9', 'assets' => array() ) ) ); + // Feed the updater a known release through its own cache instead of the // network: what matters here is the shape handed to WordPress, and a test // that needs the internet fails for reasons that have nothing to do with it.