From fc1887149dfdcaac372b74f34b0b0078fa77e234 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 11 Aug 2026 16:51:43 -0500 Subject: [PATCH 1/5] Plugin Directory: Block a release when a security scan reports a high risk score. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security scan callbacks now carry a max_risk_score and a bounded findings array. A completed scan at or above the threshold blocks the scanned release pending review — the previously served version keeps being served — and the plugin review team receives the findings as context via an internal note, a Slack alert, and a stored evidence snapshot. A verdict for a version already served, or superseded by a newer release, can't un-ship anything and stays advisory. Scanner retries are acknowledged idempotently under a canonical digest: an identical retry repeats no effects, and a completed verdict supersedes an earlier failure report for the same scan. Finding fields beyond the risk score are optional per the callback contract and read defensively. Co-Authored-By: Claude Fable 5 --- .../jobs/class-plugin-scan-gandalf.php | 220 +++++- .../tests/Gandalf_Scan_Endpoint_Test.php | 41 ++ .../tests/Security_Scan_Block_Test.php | 645 ++++++++++++++++++ 3 files changed, 870 insertions(+), 36 deletions(-) create mode 100644 wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Security_Scan_Block_Test.php 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 68274af6ec..038fe4870d 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 ) { - self::notify_slack( - $plugin, - [ - 'version' => $pending_record['version'], - 'release_ref' => $pending_record['release_ref'], - 'findings_count' => $data['findings_count'], - 'severity_counts' => $data['severity_counts'], - 'verdict_hash' => $data['verdict_hash'], - 'report_url' => $data['report_url'], - 'findings' => is_array( $data['findings'] ?? null ) ? array_filter( $data['findings'], 'is_array' ) : [], - 'max_risk_score' => $data['max_risk_score'] ?? null, - ] - ); + $record = [ + 'scan_id' => $scan_id, + 'version' => $pending_record['version'], + 'release_ref' => $pending_record['release_ref'], + 'completed_at' => $data['completed_at'] ?? time(), + 'verdict_hash' => $data['verdict_hash'], + 'findings_count' => $data['findings_count'], + 'severity_counts' => $data['severity_counts'], + 'max_risk_score' => (float) $data['max_risk_score'], + 'report_url' => $data['report_url'], + 'action' => 'advisory', + + /* + * Only the ten highest-risk findings ever surface (Slack shows five, + * the review note ten), and snippets and explanations are only in + * the scan report; storing more would bloat a post meta row that is + * loaded on every plugin page view. + */ + 'findings' => array_map( + static function ( $finding ) { + unset( $finding['code_snippet'], $finding['explanation'] ); + return $finding; + }, + self::top_findings( $data['findings'], 10 ) + ), + ]; + + /** + * Filters the risk score at which a completed security scan blocks the release. + * + * @param float $threshold The block threshold, from 0 to 10. + * @param \WP_Post $plugin The plugin post. + */ + $threshold = (float) apply_filters( 'wporg_plugins_security_scan_block_risk_score', self::BLOCK_RISK_SCORE, $plugin ); + + if ( $record['max_risk_score'] >= $threshold && self::block_release( $plugin, $record ) ) { + $record['action'] = 'blocked'; + + self::record_review_note( $plugin, $record ); + } + + // A retry of a partially processed delivery must not downgrade the recorded action. + $previous_result = get_post_meta( $plugin->ID, self::LAST_RESULT_META_KEY, true ); + if ( is_array( $previous_result ) && ( $previous_result['scan_id'] ?? '' ) === $scan_id && 'advisory' === $record['action'] ) { + $record['action'] = $previous_result['action'] ?? 'advisory'; + } + + // Persist the bounded evidence snapshot and the action taken on it. + update_post_meta( $plugin->ID, self::LAST_RESULT_META_KEY, $record ); + + if ( $record['findings_count'] > 0 || 'advisory' !== $record['action'] ) { + self::notify_slack( $plugin, $record ); } } else { self::record_last_error( $plugin, $data['error']['kind'], $data['error']['message'], $scan_id ); @@ -304,6 +353,82 @@ protected static function ksort_deep( &$data ) { unset( $value ); } + /** + * Block the scanned release, once the verdict is known to still apply to it. + * + * A verdict for a version that is no longer the plugin's current release, + * or that is already being served, can't un-ship anything — blocking is + * refused and the result stays advisory. + * + * @param \WP_Post $plugin The plugin post. + * @param array $record The completed scan record. + * @return bool Whether the release was blocked. + */ + protected static function block_release( $plugin, $record ) { + // A newer release landed since this scan was dispatched; the scanned version is moot. + if ( (string) get_post_meta( $plugin->ID, 'version', true ) !== (string) $record['version'] ) { + return false; + } + + return API_Update_Updater::block_release( + $plugin->post_name, + [ + 'scan_id' => $record['scan_id'], + 'risk_score' => $record['max_risk_score'], + ] + ); + } + + /** + * Leave an internal note with the scan findings for the plugin review team. + * + * @param \WP_Post $plugin The plugin post. + * @param array $record The completed scan record. + */ + protected static function record_review_note( $plugin, $record ) { + $note = sprintf( + 'Automatically blocked version %s (%s) from being served: security scan %s reported a maximum risk score of %s. Force-release to serve it.', + esc_html( $record['version'] ), + esc_html( $record['release_ref'] ), + esc_html( $record['scan_id'] ), + esc_html( $record['max_risk_score'] ) + ); + + $note .= '

Findings:'; + foreach ( self::top_findings( $record['findings'], 10 ) as $finding ) { + $note .= sprintf( + '
%s — %s', + esc_html( number_format( (float) $finding['risk_score'], 1 ) ), + esc_html( self::excerpt( $finding['title'] ?? '', 200 ) ) + ); + + if ( ! empty( $finding['file_path'] ) ) { + $note .= sprintf( + '
  %s%s', + esc_html( $finding['file_path'] ), + empty( $finding['line'] ) ? '' : ':' . (int) $finding['line'] + ); + } + + // The contract only requires a finding's risk_score; read the rest defensively. + $investigation = $finding['investigation'] ?? []; + if ( 'completed' === ( $investigation['status'] ?? '' ) && in_array( $investigation['result'] ?? '', [ 'reproduced', 'conditional' ], true ) ) { + $note .= sprintf( + '
  Investigation (%s): %s', + esc_html( $investigation['result'] ), + esc_html( self::excerpt( $investigation['summary'] ?? '', 200 ) ) + ); + } + } + + $note .= '

Report: ' . esc_url( $record['report_url'] ); + + $wordpressdotorg = get_user_by( 'slug', 'wordpressdotorg' ); + + // wp_insert_comment() unslashes; slash so backslashes in finding strings survive. + Tools::audit_log( wp_slash( $note ), $plugin, $wordpressdotorg ? $wordpressdotorg : false ); + } + /** * Record a valid-secret callback that failed validation. * @@ -342,7 +467,7 @@ protected static function dispatch_failed( $plugin, $request_data, $message, $ki * Notify Slack about a Gandalf scan with findings. * * @param \WP_Post $plugin The plugin post. - * @param array $record The completed scan summary. + * @param array $record The completed scan record. */ protected static function notify_slack( $plugin, $record ) { if ( empty( $record['verdict_hash'] ) ) { @@ -356,7 +481,8 @@ protected static function notify_slack( $plugin, $record ) { } } - if ( isset( $already_notified[ $record['verdict_hash'] ] ) ) { + // Release blocks always alert; only advisory results deduplicate. + if ( 'advisory' === $record['action'] && isset( $already_notified[ $record['verdict_hash'] ] ) ) { update_post_meta( $plugin->ID, self::NOTIFIED_META_KEY, $already_notified ); return; } @@ -393,11 +519,16 @@ protected static function notify_slack( $plugin, $record ) { $meta_links .= ' · ' . htmlspecialchars( $record['release_ref'], ENT_NOQUOTES ); } + $summary_text = sprintf( '%s · %s', 1 === $findings_count ? '*1 finding*' : "*{$findings_count} findings*", $install_text ); + if ( isset( $record['max_risk_score'] ) ) { + $summary_text .= sprintf( ' · max risk %s', number_format( (float) $record['max_risk_score'], 1 ) ); + } + $summary = [ 'type' => 'section', 'text' => [ 'type' => 'mrkdwn', - 'text' => sprintf( '%s · %s', 1 === $findings_count ? '*1 finding*' : "*{$findings_count} findings*", $install_text ), + 'text' => $summary_text, ], ]; @@ -422,14 +553,25 @@ protected static function notify_slack( $plugin, $record ) { 'text' => self::excerpt( trim( $title . ' ' . $record['version'] ), 150 ), ], ], - $summary, - [ - 'type' => 'context', - 'elements' => [ - [ - 'type' => 'mrkdwn', - 'text' => $meta_links, - ], + ]; + + if ( 'blocked' === $record['action'] ) { + $blocks[] = [ + 'type' => 'section', + 'text' => [ + 'type' => 'mrkdwn', + 'text' => ':rotating_light: *Automatically blocked from release.*', + ], + ]; + } + + $blocks[] = $summary; + $blocks[] = [ + 'type' => 'context', + 'elements' => [ + [ + 'type' => 'mrkdwn', + 'text' => $meta_links, ], ], ]; @@ -470,12 +612,20 @@ protected static function notify_slack( $plugin, $record ) { $attachments[] = $attachment; } - $fallback = sprintf( - 'Security scan found %s in %s %s', - 1 === $findings_count ? '1 finding' : "{$findings_count} findings", - htmlspecialchars( $title, ENT_NOQUOTES ), - htmlspecialchars( $record['version'], ENT_NOQUOTES ) - ); + if ( 'blocked' === $record['action'] ) { + $fallback = sprintf( + 'Security scan automatically blocked %s %s from release', + htmlspecialchars( $title, ENT_NOQUOTES ), + htmlspecialchars( $record['version'], ENT_NOQUOTES ) + ); + } else { + $fallback = sprintf( + 'Security scan found %s in %s %s', + 1 === $findings_count ? '1 finding' : "{$findings_count} findings", + htmlspecialchars( $title, ENT_NOQUOTES ), + htmlspecialchars( $record['version'], ENT_NOQUOTES ) + ); + } if ( isset( $record['max_risk_score'] ) ) { $fallback .= sprintf( ' (max risk %s)', number_format( (float) $record['max_risk_score'], 1 ) ); } @@ -546,8 +696,6 @@ protected static function file_link( $plugin, $release_ref, $file_path, $line ) /** * Return the highest-risk findings first, bounded for display. * - * The callback orders findings by ID, not by severity. - * * @param array $findings The scan findings. * @param int $limit Maximum number of findings to return. * @return array The highest-risk findings. @@ -556,7 +704,7 @@ protected static function top_findings( $findings, $limit ) { usort( $findings, static function ( $a, $b ) { - return ( $b['risk_score'] ?? 0 ) <=> ( $a['risk_score'] ?? 0 ); + return $b['risk_score'] <=> $a['risk_score']; } ); diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Endpoint_Test.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Endpoint_Test.php index a941a8af48..e107e3d59f 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Endpoint_Test.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Gandalf_Scan_Endpoint_Test.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use WordPressdotorg\Plugin_Directory\Jobs\API_Update_Updater; use WordPressdotorg\Plugin_Directory\Jobs\Plugin_Scan_Gandalf; use WordPressdotorg\Plugin_Directory\Plugin_Directory; @@ -208,9 +209,49 @@ public function test_callback_is_accepted(): void { $this->assertSame( 200, $response->get_status() ); $this->assertSame( array( 'success' => true ), $response->get_data() ); + $snapshot = get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::LAST_RESULT_META_KEY, true ); + $this->assertSame( 'advisory', $snapshot['action'] ); + $this->assertSame( 5.5, $snapshot['max_risk_score'] ); + $this->assertCount( 3, $snapshot['findings'] ); + $this->assertEmpty( get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::PENDING_META_KEY, true ) ); } + /** + * A high-risk callback blocks the release end to end. + */ + public function test_high_risk_callback_blocks_release(): void { + update_post_meta( + $this->plugin->ID, + 'releases', + array( + array( + 'date' => time(), + 'tag' => self::VERSION, + 'version' => self::VERSION, + 'zips_built' => true, + 'zips_built_from_revision' => 0, + 'confirmations' => array(), + 'confirmed' => true, + 'confirmations_required' => 0, + 'committer' => array(), + 'revision' => array(), + 'release_delay' => DAY_IN_SECONDS, + ), + ) + ); + + $response = $this->dispatch( $this->payload( array( 'max_risk_score' => 9.8 ) ) ); + + $this->assertSame( 200, $response->get_status() ); + + $release = Plugin_Directory::get_release( get_post( $this->plugin->ID ), self::VERSION ); + $this->assertTrue( API_Update_Updater::is_release_blocked( $release ) ); + + $snapshot = get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::LAST_RESULT_META_KEY, true ); + $this->assertSame( 'blocked', $snapshot['action'] ); + } + /** * A production-shaped failure report is accepted and recorded. */ diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Security_Scan_Block_Test.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Security_Scan_Block_Test.php new file mode 100644 index 0000000000..ce7283532e --- /dev/null +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Security_Scan_Block_Test.php @@ -0,0 +1,645 @@ + 'block-test-' . ( ++self::$plugin_count ), + 'post_title' => 'Scan Block Test Plugin', + 'post_status' => 'publish', + ) + ); + + $this->assertInstanceOf( \WP_Post::class, $plugin ); + $this->plugin = $plugin; + + /* + * The stub update_source table survives across runs — the WP test + * installer only drops core tables — so clear leftovers that would + * collide with this run's plugin ID or read as a served version. + */ + global $wpdb; + $wpdb->delete( $wpdb->prefix . 'update_source', array( 'plugin_id' => $this->plugin->ID ) ); + $wpdb->delete( $wpdb->prefix . 'update_source', array( 'plugin_slug' => $this->plugin->post_name ) ); + + update_post_meta( $this->plugin->ID, 'version', self::VERSION ); + update_post_meta( $this->plugin->ID, 'stable_tag', self::VERSION ); + $this->add_pending_scan( self::SCAN_ID ); + } + + /** + * Register a pending scan on the plugin fixture. + * + * @param string $scan_id The scan ID to register. + */ + private function add_pending_scan( string $scan_id ): void { + $pending = get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::PENDING_META_KEY, true ); + $pending = is_array( $pending ) ? $pending : array(); + + $pending[ $scan_id ] = array( + 'version' => self::VERSION, + 'release_ref' => self::VERSION, + 'requested_at' => time(), + ); + + update_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::PENDING_META_KEY, $pending ); + } + + /** + * Register a release for the scanned version, still inside its cooldown window. + */ + private function stage_release(): void { + update_post_meta( + $this->plugin->ID, + 'releases', + array( + array( + 'date' => time(), + 'tag' => self::VERSION, + 'version' => self::VERSION, + 'zips_built' => true, + 'zips_built_from_revision' => 0, + 'confirmations' => array(), + 'confirmed' => true, + 'confirmations_required' => 0, + 'committer' => array(), + 'revision' => array(), + 'release_delay' => DAY_IN_SECONDS, + ), + ) + ); + } + + /** + * Stage an update_source row for a served version alongside the cooldown release. + * + * @param string $served_version The version the row serves. + */ + private function stage_cooldown_release( string $served_version = '1.0.0' ): void { + global $wpdb; + + $wpdb->insert( + $wpdb->prefix . 'update_source', + array( + 'plugin_id' => $this->plugin->ID, + 'plugin_slug' => $this->plugin->post_name, + 'available' => 1, + 'version' => $served_version, + 'stable_tag' => $served_version, + 'plugin_name' => $this->plugin->post_title, + 'requires_plugins' => '', + 'last_updated' => $this->plugin->post_modified, + ) + ); + + $this->stage_release(); + } + + /** + * Build a finding entry matching the callback contract. + * + * @param float $risk_score The finding risk score. + * @param array $overrides Fields to override. + * @return array The finding. + */ + private function finding( float $risk_score, array $overrides = array() ): array { + return array_merge( + array( + 'id' => 'finding-' . md5( (string) $risk_score ), + 'ref' => 'prompt-security.supply_chain.remote_controlled_code', + 'title' => 'Remote response controls a PHP callable ', + 'severity' => 'error', + 'file_path' => 'includes/class-admin.php', + 'line' => 688, + 'code_snippet' => '$clean = $this->write;', + 'explanation' => 'The response body reaches a callable.', + 'risk_score' => $risk_score, + 'investigation' => array( + 'status' => 'completed', + 'result' => 'reproduced', + 'summary' => 'The unauthenticated probe reached the sink.', + ), + ), + $overrides + ); + } + + /** + * Build a completed callback matching the pending scan fixture. + * + * @param array $overrides Fields to override. + * @return array The callback data. + */ + private function completed_callback( array $overrides = array() ): array { + $defaults = array( + 'status' => 'completed', + 'scan_id' => self::SCAN_ID, + 'subject_type' => 'plugin', + 'slug' => $this->plugin->post_name, + 'version' => self::VERSION, + 'release_ref' => self::VERSION, + 'completed_at' => time(), + 'verdict_hash' => 'f71c3d944050095a4e2e20f9ee8a7c9a', + 'findings_count' => 2, + 'findings' => array( $this->finding( 9.8 ), $this->finding( 5.2 ) ), + 'max_risk_score' => 9.8, + 'severity_counts' => array( 'error' => 2 ), + 'scanner_version' => '0.3.0', + 'report_url' => 'https://scanner.example/runs/' . self::SCAN_ID, + ); + + return array_merge( $defaults, $overrides ); + } + + /** + * Build a failed callback matching the pending scan fixture. + * + * @param array $overrides Fields to override. + * @return array The callback data. + */ + private function failed_callback( array $overrides = array() ): array { + $defaults = array( + 'status' => 'failed', + 'scan_id' => self::SCAN_ID, + 'subject_type' => 'plugin', + 'slug' => $this->plugin->post_name, + 'version' => self::VERSION, + 'release_ref' => self::VERSION, + 'completed_at' => time(), + 'report_url' => 'https://scanner.example/runs/' . self::SCAN_ID, + 'error' => array( + 'kind' => 'timeout', + 'message' => 'Scan exceeded the runtime deadline.', + ), + ); + + return array_merge( $defaults, $overrides ); + } + + /** + * Fetch the internal notes recorded on the plugin fixture. + * + * @return array The internal note comments. + */ + private function get_internal_notes(): array { + return get_comments( + array( + 'post_id' => $this->plugin->ID, + 'type' => 'internal-note', + ) + ); + } + + /** + * Fetch the release record for the scanned version. + * + * @return array|false The release, or false when none exists. + */ + private function get_release() { + return Plugin_Directory::get_release( get_post( $this->plugin->ID ), self::VERSION ); + } + + /** + * The version currently served from `update_source`. + * + * @return string The served version, or an empty string when none is served. + */ + private function get_served_version(): string { + return (string) ( API_Update_Updater::get_served_release( $this->plugin->post_name )->version ?? '' ); + } + + /** + * A completed scan at the block threshold holds the release, not the plugin. + */ + public function test_high_risk_scan_blocks_release(): void { + $this->stage_release(); + + $result = Plugin_Scan_Gandalf::handle_callback( $this->plugin, $this->completed_callback() ); + + $this->assertTrue( $result ); + $this->assertTrue( API_Update_Updater::is_release_blocked( $this->get_release() ) ); + $this->assertSame( 'publish', get_post( $this->plugin->ID )->post_status ); + + $block = $this->get_release()['release_block']; + $this->assertSame( self::SCAN_ID, $block['scan_id'] ); + $this->assertSame( 9.8, $block['risk_score'] ); + $this->assertNotEmpty( $block['blocked_at'] ); + + $snapshot = get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::LAST_RESULT_META_KEY, true ); + $this->assertSame( 'blocked', $snapshot['action'] ); + $this->assertSame( 9.8, $snapshot['max_risk_score'] ); + $this->assertCount( 2, $snapshot['findings'] ); + + $consumed = get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::CONSUMED_META_KEY, true ); + $this->assertArrayHasKey( self::SCAN_ID, $consumed ); + + $this->assertEmpty( get_post_meta( $this->plugin->ID, Plugin_Scan_Gandalf::PENDING_META_KEY, true ) ); + } + + /** + * A block leaves an internal note with the escaped findings for reviewers. + */ + public function test_block_leaves_findings_note(): void { + $this->stage_release(); + + Plugin_Scan_Gandalf::handle_callback( $this->plugin, $this->completed_callback() ); + + $notes = $this->get_internal_notes(); + $this->assertCount( 1, $notes ); + + $note = $notes[0]->comment_content; + $this->assertStringContainsString( self::SCAN_ID, $note ); + $this->assertStringContainsString( 'Automatically blocked version ' . self::VERSION, $note ); + $this->assertStringContainsString( '<script>alert(1)</script>', $note ); + $this->assertStringNotContainsString( '