From c1edf5b11e55d0275452190d9db09a5494a1ab02 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Thu, 16 Jul 2026 16:21:46 -0500 Subject: [PATCH 1/3] Plugin Directory: Fix the test environment so DB and REST tests can run. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test environment was missing three things the development environment already had, which between them made whole classes of test impossible: - `update_source` was never created. `after-start.sh` imports `database-tables.sql` for development; the test script did not, so anything touching the update API's table failed. - `wp-content/env-bin` was not mapped, so `database-tables.sql` was not reachable from the test container to import in the first place. - `PLUGINS_TABLE_PREFIX` was undefined. `SVN_Access` reads it at construction, so `API\Base::load_routes()` fatalled and *no* `plugins/v1` route was registered — every REST request in the test environment 404'd. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019ngq7zaYBfv2zS8dnjFoUG --- environments/plugin-directory/.wp-env.test.json | 6 ++++++ environments/plugin-directory/bin/after-start-test.sh | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/environments/plugin-directory/.wp-env.test.json b/environments/plugin-directory/.wp-env.test.json index 6bffb7da9c..145db9cad1 100644 --- a/environments/plugin-directory/.wp-env.test.json +++ b/environments/plugin-directory/.wp-env.test.json @@ -4,7 +4,13 @@ "plugins": [ "../wordpress.org/public_html/wp-content/plugins/plugin-directory" ], + "mappings": { + "wp-content/env-bin": "./plugin-directory/bin" + }, "lifecycleScripts": { "afterStart": "bash plugin-directory/bin/after-start-test.sh" + }, + "config": { + "PLUGINS_TABLE_PREFIX": "wp_" } } diff --git a/environments/plugin-directory/bin/after-start-test.sh b/environments/plugin-directory/bin/after-start-test.sh index 13d416d04a..b68d50afcb 100755 --- a/environments/plugin-directory/bin/after-start-test.sh +++ b/environments/plugin-directory/bin/after-start-test.sh @@ -1,7 +1,7 @@ #!/bin/bash # # Runs after wp-env start for the test environment. -# Installs PHPUnit 11 and Yoast polyfills in the test container. +# Installs PHPUnit 11 and Yoast polyfills, and creates the stub tables tests read. # CONFIG="--config plugin-directory/.wp-env.test.json" @@ -10,3 +10,7 @@ RUN="npx wp-env $CONFIG run tests-cli" echo "Installing PHPUnit 11 and polyfills..." $RUN composer global require -W phpunit/phpunit:^11.0 2>&1 $RUN composer require --dev yoast/phpunit-polyfills:^4.0 --working-dir=/wordpress-phpunit 2>&1 + +# Create stub database tables that exist outside WordPress on production. +echo "Creating stub database tables..." +$RUN -- wp db import wp-content/env-bin/database-tables.sql From 6e11272434bc867adb825af4fa2ab9bb20be0826 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Thu, 16 Jul 2026 16:22:02 -0500 Subject: [PATCH 2/3] Plugin Directory: Serve a release immediately when Gandalf reports no findings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release delay exists to give scanners and humans a window to flag a bad release before it reaches sites. Until now the window was spent in full on every release, including the overwhelming majority that a scanner has already looked at and cleared. This closes that loop: when a Gandalf scan completes with no findings, the version it scanned stops waiting and starts being served. `force_release()` and the new path do the same thing — clear a release's delay and write it to `update_source` — so they collapse into one `API_Update_Updater::serve_release_now( $slug, $version, $reason, $user )`. What separates a reviewer force-releasing from a clean scan is recorded in `$reason`, not in the behaviour, and every such release now logs in one greppable format. The version is passed explicitly and re-checked against the plugin's current one. Both callers act on a version they evaluated earlier — a reviewer reading the metabox, a scanner reading a ZIP — and a newer commit can land in between; serving a stale version would skip the wait on a release nobody evaluated, which is precisely what the delay exists to cover. The metabox did this check itself and `force_release()` did not; now there is one implementation. Callback payload validation is restored, as an args schema on the route. #634 dropped it on the grounds that the scan verdict was advisory, where the worst case was a wrong Slack message. That no longer holds now that the verdict serves releases: `(int) 'abc'` is 0, and a `findings_count` that is absent or malformed would read as a clean scan and publish. We validate what we act on, at our own boundary. `error.kind` is accepted as an open string rather than a fixed enum, so a new failure kind needs no WPORG deploy. Adds coverage for the delay lifecycle and the callback route, including the gate itself, which had none. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019ngq7zaYBfv2zS8dnjFoUG --- .../admin/metabox/class-controls.php | 14 +- .../api/routes/class-gandalf-scan.php | 88 +++- .../class-plugin-directory.php | 4 +- .../jobs/class-api-update-updater.php | 58 +-- .../jobs/class-plugin-scan-gandalf.php | 28 +- .../plugin-directory/plugin-directory.php | 9 +- .../tests/Gandalf_Scan_Callback_Test.php | 398 ++++++++++++++++++ .../tests/Release_Delay_Test.php | 319 ++++++++++++++ 8 files changed, 869 insertions(+), 49 deletions(-) create mode 100644 wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Callback_Test.php create mode 100644 wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/admin/metabox/class-controls.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/admin/metabox/class-controls.php index 3d85569f28..517255259f 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/admin/metabox/class-controls.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/admin/metabox/class-controls.php @@ -138,13 +138,6 @@ public static function save_post( $post_id ) { // and to make the security boundary explicit. check_admin_referer( 'update-post_' . $post_id ); - $version = get_post_meta( $post->ID, 'version', true ); - $submitted_version = sanitize_text_field( wp_unslash( $_POST['force_release_version'] ) ); - if ( $submitted_version !== $version ) { - // Submitted version doesn't match current — a newer commit landed since the form was rendered. - return; - } - $reason = isset( $_POST['force_release_reason'] ) ? trim( sanitize_textarea_field( wp_unslash( $_POST['force_release_reason'] ) ) ) : ''; @@ -152,7 +145,12 @@ public static function save_post( $post_id ) { return; } - API_Update_Updater::force_release( $post->post_name, $reason ); + // serve_release_now() refuses a version that's no longer current. + API_Update_Updater::serve_release_now( + $post->post_name, + sanitize_text_field( wp_unslash( $_POST['force_release_version'] ) ), + $reason + ); } /** diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php index c9fce4c5a9..d0e979568a 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php @@ -14,7 +14,7 @@ use WP_Http; /** - * Callback endpoint for advisory Gandalf scans. + * Callback endpoint for Gandalf scans. * * @package WordPressdotorg_Plugin_Directory */ @@ -31,9 +31,53 @@ public function __construct() { 'methods' => \WP_REST_Server::CREATABLE, 'callback' => [ $this, 'scan_callback' ], 'args' => [ - 'plugin_slug' => [ + 'plugin_slug' => [ 'validate_callback' => [ $this, 'validate_plugin_slug_callback' ], ], + 'scan_id' => [ + 'required' => true, + 'type' => 'string', + ], + 'status' => [ + 'required' => true, + 'type' => 'string', + 'validate_callback' => [ $this, 'validate_status_callback' ], + ], + 'version' => [ + 'required' => true, + 'type' => 'string', + ], + 'release_ref' => [ + 'required' => true, + 'type' => 'string', + ], + + /* + * A clean scan lets a release skip the rest of its delay, so the count has to + * be a real number: `(int) 'abc'` is 0, which would read as clean. The minimum + * rejects a negative count, which is a contract violation rather than a verdict. + */ + 'findings_count' => [ + 'type' => 'integer', + 'minimum' => 0, + ], + 'severity_counts' => [ + 'type' => 'object', + ], + 'verdict_hash' => [ + 'type' => 'string', + ], + 'report_url' => [ + 'type' => 'string', + 'format' => 'uri', + ], + 'error' => [ + 'type' => 'object', + 'properties' => [ + 'kind' => [ 'type' => 'string' ], + 'message' => [ 'type' => 'string' ], + ], + ], ], 'permission_callback' => function ( $request ) { return $this->permission_check_api_bearer( $request, 'WP_GANDALF_SCAN_SHARED_SECRET' ); @@ -53,11 +97,7 @@ public function scan_callback( $request ) { $data = $request->get_json_params(); if ( ! is_array( $data ) ) { - $error = new WP_Error( - 'invalid_gandalf_scan_callback', - __( 'Invalid Gandalf scan callback.', 'wporg-plugins' ), - [ 'status' => WP_Http::BAD_REQUEST ] - ); + $error = new WP_Error( 'invalid_gandalf_scan_callback', 'Invalid Gandalf scan callback.', [ 'status' => WP_Http::BAD_REQUEST ] ); Plugin_Scan_Gandalf::record_invalid_callback( $plugin, $error ); return $error; @@ -72,4 +112,38 @@ public function scan_callback( $request ) { 'success' => true, ]; } + + /** + * Validate `status`, and with it the fields that a given status has to carry. + * + * The args schema can express "findings_count must be a non-negative integer" but not + * "and it must be present when the scan completed", which is the case that matters: the + * handler reads the count to decide whether a release skips the rest of its delay. + * + * @param mixed $value The status. + * @param \WP_REST_Request $request The request. + * @param string $param The parameter name. + * @return true|WP_Error True when the status and its companion fields are usable. + */ + public function validate_status_callback( $value, $request, $param ) { + $valid = rest_validate_request_arg( $value, $request, $param ); + if ( is_wp_error( $valid ) ) { + return $valid; + } + + if ( 'completed' === $value ) { + if ( null === $request->get_param( 'findings_count' ) ) { + return new WP_Error( 'invalid_gandalf_findings_count', 'A completed Gandalf scan must report a findings_count.', [ 'status' => WP_Http::BAD_REQUEST ] ); + } + + return true; + } + + $error = $request->get_param( 'error' ); + if ( ! isset( $error['kind'], $error['message'] ) ) { + return new WP_Error( 'invalid_gandalf_scan_error', 'A Gandalf scan that did not complete must describe the error.', [ 'status' => WP_Http::BAD_REQUEST ] ); + } + + return true; + } } diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-directory.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-directory.php index 105c678dfc..c3e915e183 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-directory.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-directory.php @@ -1732,8 +1732,8 @@ public static function add_release( $plugin, $data ) { 'committer' => [], 'revision' => [], // Captures the release cooldown active at creation time so future filter/constant - // changes don't retroactively affect in-flight releases. Reviewers force-release - // by overriding this to 0 — see API_Update_Updater::force_release(). + // changes don't retroactively affect in-flight releases. Overridden to 0 to serve + // a release without the wait — see API_Update_Updater::serve_release_now(). 'release_delay' => get_release_cooldown_delay( $plugin->post_name ), ]; diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php index f24821258a..9465a735ff 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php @@ -96,14 +96,9 @@ public static function update_single_plugin( $plugin_slug ) { $release_delay = (int) ( $release['release_delay'] ?? 0 ); /* - * Defer the write for new versions still inside the cooldown window. While - * deferred, the existing `update_source` row (carrying the previous version) - * continues to be served by the update API. Reviewers force-release by setting - * `release_delay = 0` on the release meta. - * - * The deferred cron fires at exactly $cooldown_until, so by definition this - * gate is false when called from cron_trigger_release() and no explicit bypass - * is needed. + * Defer new versions still inside the cooldown; the existing `update_source` row keeps + * serving the previous version meanwhile. The deferred cron fires at exactly + * $cooldown_until, so this gate is always false when called from cron_trigger_release(). */ if ( $release_delay && $existing_version !== (string) $version ) { $cooldown_until = $release_time + $release_delay; @@ -113,10 +108,7 @@ public static function update_single_plugin( $plugin_slug ) { } } - // When publishing a new version under an active cooldown, anchor `release_time` - // to now — that's the moment the version is actually available to sites. Keeps - // phased_rollout()'s `manual-updates-24hr` window measuring from public availability, - // even if the commit/confirmation was long ago because the cooldown deferred the write. + // Anchor to now: phased_rollout()'s 24h window measures from public availability. if ( $release_delay && $existing_version !== (string) $version ) { $release_time = time(); } @@ -145,8 +137,7 @@ public static function update_single_plugin( $plugin_slug ) { ); } - // The deferred event (if any) has either fired or been pre-empted by a force-release - // or status change. Clear any leftover schedule so the cron table doesn't grow. + // Clear any leftover deferred schedule so the cron table doesn't grow. wp_clear_scheduled_hook( "release_to_update_api:{$post->post_name}" ); $data = array( @@ -256,19 +247,23 @@ public static function cron_trigger_release() { } /** - * Reviewer force-release: clear the cooldown for a plugin's current version and - * write it to `update_source` immediately. Logs the action with the supplied reason. + * Write a version to `update_source` now, without waiting out its release delay. Used by + * reviewers force-releasing from wp-admin and by a clean Gandalf scan; what separates the + * two is $reason, not the behaviour. * - * Capability checks must be performed by the caller. + * $version must still be the plugin's current version — callers evaluated it earlier, and + * serving a stale one would skip the wait on a release nobody looked at. Authentication + * and capability checks are the caller's. * * @param string $plugin_slug The plugin slug. + * @param string $version The version that was evaluated. * @param string $reason Free-text reason recorded in the audit log. * @param \WP_User $user The acting user. Defaults to the current user. - * @return bool True on success. + * @return bool True when the version was served. */ - public static function force_release( $plugin_slug, $reason, $user = null ) { - if ( ! $user ) { - $user = wp_get_current_user(); + public static function serve_release_now( $plugin_slug, $version, $reason, $user = false ) { + if ( ! $version || ! $reason ) { + return false; } $post = Plugin_Directory::get_plugin_post( $plugin_slug ); @@ -276,21 +271,32 @@ public static function force_release( $plugin_slug, $reason, $user = null ) { return false; } - $version = get_post_meta( $post->ID, 'version', true ); - $release = Plugin_Directory::get_release( $post, $version ); + // A newer release landed since the caller evaluated this version. + $current_version = (string) get_post_meta( $post->ID, 'version', true ); + if ( $current_version !== (string) $version ) { + return false; + } + $release = Plugin_Directory::get_release( $post, $version ); if ( ! $release ) { return false; } + // Nothing to skip: no delay was captured at release creation, or it's already served. + $release_delay = (int) ( $release['release_delay'] ?? 0 ); + if ( ! $release_delay ) { + return false; + } + Tools::audit_log( sprintf( - 'Force-released version %s, bypassing the %d-hour release cooldown. Reason: %s', + 'Served version %1$s immediately, skipping the %2$d-hour release delay. Reason: %3$s', $version, - (int) ( $release['release_delay'] ?? 0 ) / HOUR_IN_SECONDS, + (int) round( $release_delay / HOUR_IN_SECONDS ), $reason ), - $post + $post, + $user ); Plugin_Directory::add_release( diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-plugin-scan-gandalf.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-plugin-scan-gandalf.php index aacb3d77af..68c17041d1 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-plugin-scan-gandalf.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-plugin-scan-gandalf.php @@ -1,6 +1,6 @@ 0 ) { + if ( 0 === $data['findings_count'] ) { + // serve_release_now() re-checks the version, so a release imported mid-scan keeps its delay. + API_Update_Updater::serve_release_now( + $plugin->post_name, + $pending_record['version'], + sprintf( + 'Gandalf scan %s reported no findings for %s.', + $scan_id, + $pending_record['release_ref'] + ) + ); + } else { + // Anything but a clean scan goes to a human, including a nonsensical negative count. self::notify_slack( $plugin, [ diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/plugin-directory.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/plugin-directory.php index 45b61e3a2b..14f4a077c3 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/plugin-directory.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/plugin-directory.php @@ -30,8 +30,13 @@ * and the new version being written to the `update_source` table — and so served to * sites by the api.wordpress.org plugin update-check API. The previous version remains * served until the cooldown elapses. Mitigates supply-chain attacks by giving scanners - * and humans a window to flag bad releases. Plugin reviewers can bypass the cooldown - * via the wp-admin force-release action; see Jobs\API_Update_Updater::update_single_plugin(). + * and humans a window to flag bad releases. See Jobs\API_Update_Updater::update_single_plugin() + * for the gate itself. + * + * Two things end the wait early, both via Jobs\API_Update_Updater::serve_release_now(): + * a reviewer force-releasing from wp-admin, and a Gandalf scan completing with no findings. + * The latter means the window is only spent on releases a scanner flagged or never + * reported on. * * Defers to the shared WPORG_PLUGIN_THEME_RELEASE_DELAY constant when it's defined * so the plugin and theme directories can be tuned (or disabled) in lockstep from a diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Callback_Test.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Callback_Test.php new file mode 100644 index 0000000000..0843957e4f --- /dev/null +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Callback_Test.php @@ -0,0 +1,398 @@ + 'plugin', + 'post_name' => self::SLUG, + 'post_title' => 'Gandalf Callback Test', + 'post_status' => 'publish', + 'post_modified' => current_time( 'mysql' ), + 'post_modified_gmt' => current_time( 'mysql', 1 ), + ), + true + ); + + $this->assertNotInstanceOf( WP_Error::class, $plugin_id ); + + $this->plugin = get_post( $plugin_id ); + + update_post_meta( $plugin_id, 'version', self::NEW_VERSION ); + update_post_meta( $plugin_id, 'stable_tag', self::NEW_VERSION ); + update_post_meta( $plugin_id, 'header_name', 'Gandalf Callback Test' ); + update_post_meta( $plugin_id, 'header_author', 'WordPress' ); + update_post_meta( $plugin_id, 'version_date', current_time( 'mysql', 1 ) ); + + update_post_meta( + $plugin_id, + 'releases', + array( + array( + 'date' => time(), + 'tag' => self::NEW_VERSION, + 'version' => self::NEW_VERSION, + 'zips_built' => true, + 'confirmations' => array(), + 'confirmed' => true, + 'confirmations_required' => 0, + 'committer' => array(), + 'revision' => array(), + 'release_delay' => self::DELAY, + ), + ) + ); + + $this->set_pending_scan(); + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- `update_source` lives outside WordPress; there is no API for it. + $wpdb->query( "TRUNCATE TABLE `{$wpdb->prefix}update_source`" ); + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- Seeding the version this test starts out serving. + $wpdb->insert( + $wpdb->prefix . 'update_source', + array( + 'plugin_id' => $plugin_id, + 'plugin_slug' => self::SLUG, + 'available' => 1, + 'version' => self::SERVED_VERSION, + 'last_updated' => current_time( 'mysql' ), + ) + ); + } + + /** + * Remove the plugin, its meta, any audit-log notes, and any deferred cron event. + */ + protected function tearDown(): void { + wp_clear_scheduled_hook( 'release_to_update_api:' . self::SLUG ); + + foreach ( get_comments( array( 'post_id' => $this->plugin->ID ) ) as $note ) { + wp_delete_comment( $note->comment_ID, true ); + } + + wp_delete_post( $this->plugin->ID, true ); + + parent::tearDown(); + } + + /** + * Record the in-flight scan the callback is correlated against. + */ + protected function set_pending_scan() { + update_post_meta( + $this->plugin->ID, + '_gandalf_scan_pending', + array( + self::SCAN_ID => array( + 'version' => self::NEW_VERSION, + 'release_ref' => self::NEW_VERSION, + 'requested_at' => time(), + ), + ) + ); + } + + /** + * A well-formed callback body, as Gandalf would send it. + * + * @param array $overrides Values to override. + * @return array + */ + protected function callback_body( $overrides = array() ) { + return array_merge( + array( + 'scan_id' => self::SCAN_ID, + 'status' => 'completed', + 'version' => self::NEW_VERSION, + 'release_ref' => self::NEW_VERSION, + 'findings_count' => 0, + 'severity_counts' => array(), + 'verdict_hash' => 'abc123', + 'report_url' => 'https://gandalf.wordpress.org/report/abc123', + ), + $overrides + ); + } + + /** + * POST a callback to the route, as Gandalf would. + * + * @param array $body The callback body. + * @param string|null $secret The bearer secret, or null to send no Authorization header. + * @return \WP_REST_Response + */ + protected function post_callback( $body, $secret = self::SECRET ) { + $request = new WP_REST_Request( 'POST', '/plugins/v1/plugin/' . self::SLUG . '/gandalf-scan' ); + + if ( null !== $secret ) { + $request->set_header( 'authorization', 'Bearer ' . $secret ); + } + + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $body ) ); + + return rest_do_request( $request ); + } + + /** + * The version currently served from `update_source`. + * + * @return string|null + */ + protected function get_served_version() { + global $wpdb; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- `update_source` lives outside WordPress, and a cached read would defeat the assertion. + return $wpdb->get_var( + $wpdb->prepare( + "SELECT `version` FROM `{$wpdb->prefix}update_source` WHERE `plugin_slug` = %s", + self::SLUG + ) + ); + } + + /** + * The pending scans still awaiting a callback. + * + * @return array + */ + protected function get_pending_scans() { + $pending = get_post_meta( $this->plugin->ID, '_gandalf_scan_pending', true ); + + return $pending ? $pending : array(); + } + + /** + * The headline behaviour: Gandalf reports a clean scan over the API, and the version + * it scanned stops waiting and starts being served. + */ + public function test_a_callback_with_no_findings_serves_the_release() { + $response = $this->post_callback( $this->callback_body() ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertSame( self::NEW_VERSION, $this->get_served_version() ); + } + + /** + * A scan that reported back is no longer in flight. + */ + public function test_a_callback_with_no_findings_clears_the_pending_scan() { + $this->post_callback( $this->callback_body() ); + + $this->assertSame( array(), $this->get_pending_scans() ); + } + + /** + * The scan that ended the wait is named, so the release can be traced back to it. + */ + public function test_a_callback_with_no_findings_records_the_scan_in_the_audit_log() { + $this->post_callback( $this->callback_body() ); + + $notes = get_comments( + array( + 'post_id' => $this->plugin->ID, + 'type' => 'internal-note', + ) + ); + + $this->assertCount( 1, $notes ); + $this->assertStringContainsString( self::SCAN_ID, $notes[0]->comment_content ); + } + + /** + * Findings are the case the delay exists for, so the release keeps waiting. + */ + public function test_a_callback_with_findings_leaves_the_release_waiting() { + $response = $this->post_callback( $this->callback_body( array( 'findings_count' => 3 ) ) ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * Serving a release early is authenticated, not open to anyone who finds the URL. + */ + public function test_a_callback_without_the_shared_secret_is_rejected() { + $response = $this->post_callback( $this->callback_body(), null ); + + $this->assertSame( 401, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * The secret is compared, not merely required. + */ + public function test_a_callback_with_the_wrong_shared_secret_is_rejected() { + $response = $this->post_callback( $this->callback_body(), 'not-the-secret' ); + + $this->assertSame( 401, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * A verdict for a scan that was never dispatched acts on nothing. + */ + public function test_a_callback_for_an_unknown_scan_is_rejected() { + $response = $this->post_callback( $this->callback_body( array( 'scan_id' => 'ffffffff-0000-4000-8000-ffffffffffff' ) ) ); + + $this->assertSame( 400, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * The callback is correlated against the pending record, so a verdict that arrives for + * a different version than the one dispatched can't serve anything. + */ + public function test_a_callback_for_a_different_version_is_rejected() { + $response = $this->post_callback( $this->callback_body( array( 'version' => '9.9' ) ) ); + + $this->assertSame( 400, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + $this->assertArrayHasKey( self::SCAN_ID, $this->get_pending_scans() ); + } + + /** + * `(int) 'abc'` is 0, so a findings_count that isn't a number must be rejected rather + * than read as a clean scan. + * + * @dataProvider data_unusable_findings_counts + * @param mixed $findings_count The value Gandalf sent. + */ + public function test_a_callback_without_a_usable_findings_count_is_rejected( $findings_count ) { + $body = $this->callback_body(); + $body['findings_count'] = $findings_count; + + $response = $this->post_callback( $body ); + + $this->assertSame( 400, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + $this->assertArrayHasKey( self::SCAN_ID, $this->get_pending_scans() ); + } + + /** + * Values a findings_count could arrive as if Gandalf's payload drifted. + * + * @return array + */ + public static function data_unusable_findings_counts() { + return array( + 'non-numeric string' => array( 'abc' ), + 'null' => array( null ), + 'boolean' => array( false ), + 'array' => array( array() ), + ); + } + + /** + * A missing findings_count is the realistic drift case: `null > 0` is false, so a plain + * `else` would have read it as clean. + */ + public function test_a_callback_with_no_findings_count_at_all_is_rejected() { + $body = $this->callback_body(); + unset( $body['findings_count'] ); + + $response = $this->post_callback( $body ); + + $this->assertSame( 400, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * A scan that couldn't reach a verdict leaves the delay to run its course. + */ + public function test_a_failed_scan_leaves_the_release_waiting_and_records_the_error() { + $response = $this->post_callback( + $this->callback_body( + array( + 'status' => 'failed', + 'error' => array( + 'kind' => 'zip_unreadable', + 'message' => 'Could not read the plugin ZIP.', + ), + ) + ) + ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + + $error = get_post_meta( $this->plugin->ID, '_gandalf_scan_last_error', true ); + $this->assertSame( 'zip_unreadable', $error['kind'] ); + } +} diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php new file mode 100644 index 0000000000..23bc94c748 --- /dev/null +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php @@ -0,0 +1,319 @@ + 'plugin', + 'post_name' => self::SLUG, + 'post_title' => 'Release Delay Test', + 'post_status' => 'publish', + 'post_modified' => current_time( 'mysql' ), + 'post_modified_gmt' => current_time( 'mysql', 1 ), + ), + true + ); + + $this->assertNotInstanceOf( WP_Error::class, $plugin_id ); + + $this->plugin = get_post( $plugin_id ); + + update_post_meta( $plugin_id, 'version', self::NEW_VERSION ); + update_post_meta( $plugin_id, 'stable_tag', self::NEW_VERSION ); + update_post_meta( $plugin_id, 'header_name', 'Release Delay Test' ); + update_post_meta( $plugin_id, 'header_author', 'WordPress' ); + + $this->set_committed_at( time() ); + $this->set_releases( array( $this->release() ) ); + + $wpdb->query( "TRUNCATE TABLE `{$wpdb->prefix}update_source`" ); + $this->serve( self::SERVED_VERSION ); + } + + /** + * Remove the plugin, its meta, any audit-log notes, and any deferred cron event. There's + * no transaction to roll back without WP_UnitTestCase, so state would otherwise leak. + */ + protected function tearDown(): void { + wp_clear_scheduled_hook( 'release_to_update_api:' . self::SLUG ); + + foreach ( get_comments( array( 'post_id' => $this->plugin->ID ) ) as $note ) { + wp_delete_comment( $note->comment_ID, true ); + } + + wp_delete_post( $this->plugin->ID, true ); + + parent::tearDown(); + } + + /** + * A complete release row. get_releases() reads keys well beyond the ones under test, + * so partial rows warn rather than fail, which is easy to miss. + * + * @param array $overrides Values to override on the default release. + * @return array + */ + protected function release( $overrides = array() ) { + return array_merge( + array( + 'date' => time(), + 'tag' => self::NEW_VERSION, + 'version' => self::NEW_VERSION, + 'zips_built' => true, + 'confirmations' => array(), + 'confirmed' => true, + 'confirmations_required' => 0, + 'committer' => array(), + 'revision' => array(), + 'release_delay' => self::DELAY, + ), + $overrides + ); + } + + /** + * Seed the releases meta directly: get_releases() otherwise falls back to + * prefill_releases_meta(), which reaches out to SVN. + * + * @param array $releases The releases to store. + */ + protected function set_releases( $releases ) { + update_post_meta( $this->plugin->ID, 'releases', $releases ); + } + + /** + * Move the commit time the delay is measured from. compute_release_time() reads + * `version_date`, so this decides whether the window is still open. + * + * @param int $timestamp When the version was committed. + */ + protected function set_committed_at( $timestamp ) { + update_post_meta( $this->plugin->ID, 'version_date', gmdate( 'Y-m-d H:i:s', $timestamp ) ); + } + + /** + * Put a version into `update_source`, standing in for a previous release. + * + * @param string $version The version to serve. + */ + protected function serve( $version ) { + global $wpdb; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- `update_source` lives outside WordPress; there is no API for it. + $wpdb->insert( + $wpdb->prefix . 'update_source', + array( + 'plugin_id' => $this->plugin->ID, + 'plugin_slug' => self::SLUG, + 'available' => 1, + 'version' => $version, + 'last_updated' => current_time( 'mysql' ), + ) + ); + } + + /** + * The delay currently stored against a release tag. + * + * @param string $tag The release tag. + * @return int|null Delay in seconds, or null when the tag isn't found. + */ + protected function get_stored_delay( $tag ) { + foreach ( (array) get_post_meta( $this->plugin->ID, 'releases', true ) as $release ) { + if ( $tag === $release['tag'] ) { + return $release['release_delay']; + } + } + + return null; + } + + /** + * The version currently served from `update_source`. + * + * @return string|null + */ + protected function get_served_version() { + global $wpdb; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- `update_source` lives outside WordPress, and a cached read would defeat the assertion. + return $wpdb->get_var( + $wpdb->prepare( + "SELECT `version` FROM `{$wpdb->prefix}update_source` WHERE `plugin_slug` = %s", + self::SLUG + ) + ); + } + + /** + * The previous version keeps being served while the new one waits. + */ + public function test_holds_back_a_version_that_is_still_inside_its_delay() { + API_Update_Updater::update_single_plugin( self::SLUG ); + + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * The deferred write is scheduled for the moment the delay runs out. + */ + public function test_schedules_the_held_version_to_be_served_when_its_delay_elapses() { + $committed_at = time(); + $this->set_committed_at( $committed_at ); + + API_Update_Updater::update_single_plugin( self::SLUG ); + + $this->assertSame( + $committed_at + self::DELAY, + wp_next_scheduled( 'release_to_update_api:' . self::SLUG ) + ); + } + + /** + * Once the window has passed the version is served and the schedule is cleared. + */ + public function test_serves_a_version_whose_delay_has_elapsed() { + $this->set_committed_at( time() - ( self::DELAY * 2 ) ); + + API_Update_Updater::update_single_plugin( self::SLUG ); + + $this->assertSame( self::NEW_VERSION, $this->get_served_version() ); + $this->assertFalse( wp_next_scheduled( 'release_to_update_api:' . self::SLUG ) ); + } + + /** + * With the delay disabled, releases are served as they were before it existed. + */ + public function test_serves_a_version_that_has_no_delay_at_all() { + $this->set_releases( array( $this->release( array( 'release_delay' => 0 ) ) ) ); + + API_Update_Updater::update_single_plugin( self::SLUG ); + + $this->assertSame( self::NEW_VERSION, $this->get_served_version() ); + } + + /** + * Ending the wait early serves the version and zeroes the delay it skipped. + */ + public function test_serving_now_serves_the_version_and_clears_its_delay() { + $result = API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, 'Gandalf scan reported no findings.' ); + + $this->assertTrue( $result ); + $this->assertSame( 0, $this->get_stored_delay( self::NEW_VERSION ) ); + $this->assertSame( self::NEW_VERSION, $this->get_served_version() ); + } + + /** + * Whatever ended the wait is recorded, since the delay existed for a reason. + */ + public function test_serving_now_records_the_reason_in_the_audit_log() { + API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, 'Gandalf scan reported no findings.' ); + + $notes = get_comments( + array( + 'post_id' => $this->plugin->ID, + 'type' => 'internal-note', + ) + ); + + $this->assertCount( 1, $notes ); + $this->assertStringContainsString( 'Gandalf scan reported no findings.', $notes[0]->comment_content ); + $this->assertStringContainsString( '24-hour release delay', $notes[0]->comment_content ); + } + + /** + * The guard that matters: when a newer commit lands while a scan or review is in + * flight, the verdict on the older version must not skip the newer one's delay. + */ + public function test_serving_now_refuses_a_version_that_is_no_longer_current() { + update_post_meta( $this->plugin->ID, 'version', '3.0' ); + + $result = API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, 'Gandalf scan reported no findings.' ); + + $this->assertFalse( $result ); + $this->assertSame( self::DELAY, $this->get_stored_delay( self::NEW_VERSION ) ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * There's no wait to end when no delay was captured at release creation. + */ + public function test_serving_now_refuses_a_release_that_has_no_delay() { + $this->set_releases( array( $this->release( array( 'release_delay' => 0 ) ) ) ); + + $result = API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, 'Gandalf scan reported no findings.' ); + + $this->assertFalse( $result ); + $this->assertSame( self::SERVED_VERSION, $this->get_served_version() ); + } + + /** + * An unknown slug is refused rather than treated as a missing plugin to create. + */ + public function test_serving_now_refuses_an_unknown_plugin() { + $this->assertFalse( API_Update_Updater::serve_release_now( 'no-such-plugin-here', self::NEW_VERSION, 'Gandalf scan reported no findings.' ) ); + } + + /** + * A release is never served without both a version to check and a reason to log. + */ + public function test_serving_now_refuses_an_empty_version_or_reason() { + $this->assertFalse( API_Update_Updater::serve_release_now( self::SLUG, '', 'Gandalf scan reported no findings.' ) ); + $this->assertFalse( API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, '' ) ); + $this->assertSame( self::DELAY, $this->get_stored_delay( self::NEW_VERSION ) ); + } +} From 27cddb6eded53481c9239765df05268fb95151f6 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Fri, 17 Jul 2026 13:58:27 -0500 Subject: [PATCH 3/3] Plugin Directory: Keep the manual-updates window measured from publication when serving early. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Serving a release early via serve_release_now() zeroed its release_delay before update_single_plugin() ran, which disabled the guard that anchors release_time to the moment the version goes public. release_time fell back to the commit time, silently shortening (or collapsing) the author-set manual-updates-24hr window that phased_rollout() measures from it. Anchor release_time whenever a version newly enters update_source, not only while a delay is active, so an expedited release still dates from publication. A clean scan arriving after the delay already elapsed now leaves the served version — and its running window — untouched. Extracts get_served_version() to share the update_source lookup that both paths need. Co-Authored-By: Claude Opus 4.8 --- .../jobs/class-api-update-updater.php | 35 ++++++++--- .../tests/Release_Delay_Test.php | 59 +++++++++++++++++++ 2 files changed, 85 insertions(+), 9 deletions(-) diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php index 9465a735ff..bd74a27d61 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-api-update-updater.php @@ -86,12 +86,7 @@ public static function update_single_plugin( $plugin_slug ) { $requires_plugins = get_post_meta( $post->ID, 'requires_plugins', true ); $release = Plugin_Directory::get_release( $post, $version ); $release_time = self::compute_release_time( $post, $release ); - $existing_version = (string) $wpdb->get_var( - $wpdb->prepare( - "SELECT version FROM {$wpdb->prefix}update_source WHERE plugin_slug = %s", - $post->post_name - ) - ); + $existing_version = self::get_served_version( $post->post_name ); $release_delay = (int) ( $release['release_delay'] ?? 0 ); @@ -108,8 +103,8 @@ public static function update_single_plugin( $plugin_slug ) { } } - // Anchor to now: phased_rollout()'s 24h window measures from public availability. - if ( $release_delay && $existing_version !== (string) $version ) { + // Anchor to now: phased_rollout()'s 24h window measures from public availability, not the commit. + if ( $existing_version !== (string) $version ) { $release_time = time(); } @@ -199,6 +194,23 @@ public static function update_single_plugin( $plugin_slug ) { return true; } + /** + * The version currently served from `update_source`. + * + * @param string $plugin_slug The plugin slug. + * @return string The served version, or '' when the plugin isn't in `update_source`. + */ + public static function get_served_version( $plugin_slug ) { + global $wpdb; + + return (string) $wpdb->get_var( + $wpdb->prepare( + "SELECT version FROM {$wpdb->prefix}update_source WHERE plugin_slug = %s", + $plugin_slug + ) + ); + } + /** * Determine the release timestamp for a plugin version. * @@ -282,12 +294,17 @@ public static function serve_release_now( $plugin_slug, $version, $reason, $user return false; } - // Nothing to skip: no delay was captured at release creation, or it's already served. + // Nothing to skip: no delay was captured at release creation. $release_delay = (int) ( $release['release_delay'] ?? 0 ); if ( ! $release_delay ) { return false; } + // Already served: the delay elapsed on its own; leave `update_source` and its `release_time` untouched. + if ( self::get_served_version( $post->post_name ) === (string) $version ) { + return false; + } + Tools::audit_log( sprintf( 'Served version %1$s immediately, skipping the %2$d-hour release delay. Reason: %3$s', diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php index 23bc94c748..e9d0961249 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Release_Delay_Test.php @@ -199,6 +199,28 @@ protected function get_served_version() { ); } + /** + * The `release_time` stored against the served row. phased_rollout()'s manual-updates-24hr + * window measures from this, so serving early must not move it back to the commit time. + * + * @return int|null Unix timestamp, or null when the row or field is absent. + */ + protected function get_served_release_time() { + global $wpdb; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- `update_source` lives outside WordPress, and a cached read would defeat the assertion. + $meta = $wpdb->get_var( + $wpdb->prepare( + "SELECT `meta` FROM `{$wpdb->prefix}update_source` WHERE `plugin_slug` = %s", + self::SLUG + ) + ); + + $meta = $meta ? maybe_unserialize( $meta ) : array(); + + return isset( $meta['release_time'] ) ? (int) $meta['release_time'] : null; + } + /** * The previous version keeps being served while the new one waits. */ @@ -316,4 +338,41 @@ public function test_serving_now_refuses_an_empty_version_or_reason() { $this->assertFalse( API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, '' ) ); $this->assertSame( self::DELAY, $this->get_stored_delay( self::NEW_VERSION ) ); } + + /** + * The manual-updates-24hr window must run from when the version goes public, so serving + * early stamps `release_time` at now, not at the commit that may long predate it. + */ + public function test_serving_now_dates_the_release_from_publication_not_commit() { + $this->set_committed_at( time() - ( 10 * HOUR_IN_SECONDS ) ); + + $before = time(); + API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, 'Gandalf scan reported no findings.' ); + + $this->assertGreaterThanOrEqual( $before, $this->get_served_release_time() ); + } + + /** + * A callback arriving after the delay already elapsed has nothing to skip: the served + * version and its manual-updates window are left exactly as they are. + */ + public function test_serving_now_leaves_an_already_served_version_untouched() { + global $wpdb; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery -- `update_source` lives outside WordPress; there is no API for it. + $wpdb->update( + $wpdb->prefix . 'update_source', + array( + 'version' => self::NEW_VERSION, + 'meta' => maybe_serialize( array( 'release_time' => 1000 ) ), + ), + array( 'plugin_slug' => self::SLUG ) + ); + + $result = API_Update_Updater::serve_release_now( self::SLUG, self::NEW_VERSION, 'Gandalf scan reported no findings.' ); + + $this->assertFalse( $result ); + $this->assertSame( 1000, $this->get_served_release_time() ); + $this->assertSame( self::DELAY, $this->get_stored_delay( self::NEW_VERSION ) ); + } }