Plugin Directory: Set up release blocking for high-risk security scans

environments/plugin-directory/.wp-env.test.json

@@ -12,6 +12,7 @@
 		"afterStart": "bash plugin-directory/bin/after-start-test.sh"
 	},
 	"config": {
-		"PLUGINS_TABLE_PREFIX": "wp_"
+		"PLUGINS_TABLE_PREFIX": "wp_",
+		"WP_GANDALF_SCAN_SHARED_SECRET": "local-dev-secret"
 	}
 }

wordpress.org/public_html/wp-content/plugins/plugin-directory/api/routes/class-gandalf-scan.php

@@ -44,9 +44,10 @@ public function __construct() {
 						'validate_callback' => [ $this, 'validate_plugin_slug_callback' ],
 					],
 					'status'          => [
-						'type'     => 'string',
-						'enum'     => [ 'completed', 'failed' ],
-						'required' => true,
+						'type'              => 'string',
+						'enum'              => [ 'completed', 'failed' ],
+						'required'          => true,
+						'validate_callback' => [ $this, 'validate_status_callback' ],
 					],
 					'scan_id'         => [
 						'type'      => 'string',
@@ -181,6 +182,43 @@ public function __construct() {
 		);
 	}
 
+	/**
+	 * Validate that a callback body carries the fields its status implies.
+	 *
+	 * The contract has two halves — a completed scan reports a verdict, a
+	 * failed one reports an error — and `required` cannot express either,
+	 * being unconditional. Enforcing it here keeps the whole body schema in
+	 * the route, so the callback runs only on a payload it can read directly.
+	 *
+	 * @param mixed            $value   The status value.
+	 * @param \WP_REST_Request $request The request.
+	 * @param string           $param   The parameter name.
+	 * @return true|WP_Error True when the body matches its status.
+	 */
+	public function validate_status_callback( $value, $request, $param ) {
+		$valid = rest_validate_request_arg( $value, $request, $param );
+		if ( is_wp_error( $valid ) ) {
+			return $valid;
+		}
+
+		$required = 'completed' === $value
+			? [ 'verdict_hash', 'findings_count', 'severity_counts', 'max_risk_score', 'findings', 'report_url' ]
+			: [ 'error' ];
+
+		foreach ( $required as $field ) {
+			if ( null === $request->get_param( $field ) ) {
+				return new WP_Error(
+					'rest_missing_callback_param',
+					/* translators: 1: Field name, 2: Callback status. */
+					sprintf( __( '%1$s is required for a %2$s security scan callback.', 'wporg-plugins' ), $field, $value ),
+					[ 'status' => WP_Http::BAD_REQUEST ]
+				);
+			}
+		}
+
+		return true;
+	}
+
 	/**
 	 * Receive a security scan callback.
 	 *

wordpress.org/public_html/wp-content/plugins/plugin-directory/jobs/class-plugin-scan-gandalf.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * Advisory Gandalf scan integration for plugin updates.
+ * Gandalf scan integration for plugin updates.
  *
  * @package WordPressdotorg\Plugin_Directory\Jobs
  */
@@ -9,11 +9,16 @@
 
 use WordPressdotorg\Plugin_Directory\Plugin_Directory;
 use WordPressdotorg\Plugin_Directory\Template;
+use WordPressdotorg\Plugin_Directory\Tools;
 use WP_Error;
 use WP_Http;
 
 /**
- * Sends plugin updates to Gandalf for advisory security scans.
+ * Sends plugin updates to Gandalf for security scans and acts on the results.
+ *
+ * Completed scans whose maximum risk score reaches the block threshold hold
+ * the scanned release out of the update API — the previously served version
+ * keeps being served.
  *
  * @package WordPressdotorg\Plugin_Directory\Jobs
  */
@@ -31,6 +36,9 @@ class Plugin_Scan_Gandalf {
 	/** Consumed callbacks keyed by scan_id, to acknowledge retries without repeating effects. */
 	const CONSUMED_META_KEY = '_gandalf_scan_consumed';
 
+	/** Completed scans with a max risk score at or above this have their release blocked. */
+	const BLOCK_RISK_SCORE = PHP_FLOAT_MAX;
+
 	/** Gandalf scan endpoint. */
 	const ENDPOINT = 'https://gandalf.wordpress.org/scan';
 
@@ -239,20 +247,36 @@ protected static function consume_callback( $plugin, $data ) {
 		}
 
 		if ( 'completed' === $data['status'] ) {
-			if ( $data['findings_count'] > 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',
+				'findings'        => $data['findings'],
+			];
+
+			/**
+			 * 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 );
+			}
+
+			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 +328,85 @@ 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 release that is no longer the plugin's current one, 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 ) {
+		// Whether the verdict still applies turns on the scanned tag, never the version header an author can rename inside it.
+		$current     = API_Update_Updater::get_current_release( $plugin );
+		$scanned_tag = 'trunk' === $record['release_ref'] ? 'trunk@' . $record['version'] : $record['release_ref'];
+
+		if ( ! $current || (string) ( $current['tag'] ?? '' ) !== (string) $scanned_tag ) {
+			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( number_format( (float) $record['max_risk_score'], 1 ) )
+		);
+
+		$note .= '<br><br>Findings:';
+		foreach ( self::top_findings( $record['findings'], 10 ) as $finding ) {
+			$note .= sprintf(
+				'<br>&#8226; <strong>%s</strong> &mdash; %s',
+				esc_html( number_format( (float) $finding['risk_score'], 1 ) ),
+				esc_html( self::excerpt( $finding['title'] ?? '', 200 ) )
+			);
+
+			if ( ! empty( $finding['file_path'] ) ) {
+				$note .= sprintf(
+					'<br>&nbsp;&nbsp;%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(
+					'<br>&nbsp;&nbsp;Investigation (%s): %s',
+					esc_html( $investigation['result'] ),
+					esc_html( self::excerpt( $investigation['summary'] ?? '', 200 ) )
+				);
+			}
+		}
+
+		$note .= '<br><br>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 +445,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 +459,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 +497,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 +531,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 +590,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 +674,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 +682,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'];
 			}
 		);