Secrets: Tell a failed record read apart from a missing secret.

src/wp-includes/secrets/class-wp-secrets-option-store.php

@@ -53,7 +53,19 @@ public function get( string $name, bool $network = false ) {
 		$value       = $network ? get_site_option( $option_name, null ) : get_option( $option_name, null );
 
 		if ( null === $value ) {
-			return null;
+			// The options API returns the default both when a record is absent and when
+			// its query failed, so read the row directly to tell those two apart.
+			$stored = $this->read_stored_record( $option_name, $network );
+
+			if ( is_wp_error( $stored ) ) {
+				return $stored;
+			}
+
+			if ( false === $stored ) {
+				return null;
+			}
+
+			$value = $stored;
 		}
 
 		if ( ! is_array( $value ) ) {
@@ -204,4 +216,53 @@ public function list_names( bool $network = false ) {
 	private function option_name( string $name, bool $network ): string {
 		return ( $network ? self::NETWORK_PREFIX : self::SITE_PREFIX ) . $name;
 	}
+
+	/**
+	 * Reads a secret's stored row directly, bypassing the options cache.
+	 *
+	 * Failure is taken from wpdb::query(), which returns false on every failed read
+	 * and a row count on every successful one. wpdb::$last_error cannot be used for
+	 * this: a lost connection returns before it is set, and the early returns for an
+	 * unready handle or an emptied `query` filter leave the previous query's value
+	 * in place.
+	 *
+	 * @since 7.2.0
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 *
+	 * @param string $option_name The record's option name.
+	 * @param bool   $network     Whether this is a network-scope secret.
+	 * @return mixed|false|WP_Error The stored value, false if no row exists, or
+	 *                              WP_Error if the read failed.
+	 */
+	private function read_stored_record( string $option_name, bool $network ) {
+		global $wpdb;
+
+		if ( $network && is_multisite() ) {
+			$sql = $wpdb->prepare(
+				"SELECT meta_value AS record FROM {$wpdb->sitemeta} WHERE meta_key = %s AND site_id = %d LIMIT 1",
+				$option_name,
+				get_current_network_id()
+			);
+		} else {
+			$sql = $wpdb->prepare(
+				"SELECT option_value AS record FROM {$wpdb->options} WHERE option_name = %s LIMIT 1",
+				$option_name
+			);
+		}
+
+		// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- $sql is prepared in both branches above.
+		if ( false === $wpdb->query( $sql ) ) {
+			return new WP_Error(
+				WP_SECRETS_ERROR_STORE_UNAVAILABLE,
+				__( 'The secret record could not be read.' )
+			);
+		}
+
+		if ( empty( $wpdb->last_result ) ) {
+			return false;
+		}
+
+		return maybe_unserialize( $wpdb->last_result[0]->record );
+	}
 }

tests/phpunit/tests/secrets/threeStateContract.php

@@ -200,4 +200,77 @@ public function test_exists_and_decrypts_is_a_wp_secret(): void {
 		$this->assertNotWPError( $result );
 		$this->assertInstanceOf( WP_Secret::class, $result );
 	}
+
+	/**
+	 * @ticket 66424
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 */
+	public function test_a_failed_record_read_is_wp_error_not_null(): void {
+		global $wpdb;
+
+		wp_set_secret( 'myplugin/api-key', 'value' );
+		wp_cache_delete( '_wp_secret_myplugin/api-key', 'options' );
+
+		$suppress = $wpdb->suppress_errors( true );
+		add_filter( 'query', array( $this, 'fail_reads_of_the_api_key_record' ) );
+
+		$result = wp_get_secret( 'myplugin/api-key' );
+
+		remove_filter( 'query', array( $this, 'fail_reads_of_the_api_key_record' ) );
+		$wpdb->suppress_errors( $suppress );
+
+		$this->assertNotNull( $result );
+		$this->assertWPError( $result );
+		$this->assertSame( WP_SECRETS_ERROR_STORE_UNAVAILABLE, $result->get_error_code() );
+	}
+
+	/**
+	 * A write during a failed read must not treat the secret as new, which would
+	 * drop the version a rotation in progress still depends on.
+	 *
+	 * @ticket 66424
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 */
+	public function test_a_failed_record_read_does_not_let_a_write_report_success(): void {
+		global $wpdb;
+
+		wp_set_secret( 'myplugin/api-key', 'one' );
+		wp_set_secret( 'myplugin/api-key', 'two' );
+		wp_cache_delete( '_wp_secret_myplugin/api-key', 'options' );
+
+		$suppress = $wpdb->suppress_errors( true );
+		add_filter( 'query', array( $this, 'fail_reads_of_the_api_key_record' ) );
+
+		$written = wp_set_secret( 'myplugin/api-key', 'three' );
+
+		remove_filter( 'query', array( $this, 'fail_reads_of_the_api_key_record' ) );
+		$wpdb->suppress_errors( $suppress );
+
+		$this->assertWPError( $written );
+		$this->assertSame( WP_SECRETS_ERROR_STORE_UNAVAILABLE, $written->get_error_code() );
+
+		$current  = wp_get_secret( 'myplugin/api-key' );
+		$previous = wp_get_secret( 'myplugin/api-key', WP_Secret_Version::PREVIOUS );
+
+		$this->assertInstanceOf( WP_Secret::class, $current );
+		$this->assertSame( 'two', $current->reveal() );
+		$this->assertInstanceOf( WP_Secret::class, $previous );
+		$this->assertSame( 'one', $previous->reveal() );
+	}
+
+	/**
+	 * Makes every read of this test's own secret record fail.
+	 *
+	 * @param string $query The query about to run.
+	 * @return string The query to run instead.
+	 */
+	public function fail_reads_of_the_api_key_record( $query ) {
+		if ( 0 === stripos( $query, 'SELECT' ) && false !== strpos( $query, '_wp_secret_myplugin/api-key' ) ) {
+			return 'SELECT this is not valid SQL';
+		}
+
+		return $query;
+	}
 }

tests/phpunit/tests/secrets/wpSecretsOptionStore.php

@@ -74,6 +74,129 @@ public function test_get_returns_an_error_when_the_stored_value_is_not_an_array(
 		$this->assertSame( WP_SECRETS_ERROR_RECORD_MALFORMED, $result->get_error_code() );
 	}
 
+	/**
+	 * @ticket 66424
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 */
+	public function test_get_returns_an_error_when_the_record_read_fails(): void {
+		global $wpdb;
+
+		$store = new WP_Secrets_Option_Store();
+		$store->set( 'myplugin/key', $this->sample_record() );
+		wp_cache_delete( '_wp_secret_myplugin/key', 'options' );
+
+		$suppress = $wpdb->suppress_errors( true );
+		add_filter( 'query', array( $this, 'fail_option_reads' ) );
+
+		$result = $store->get( 'myplugin/key' );
+
+		remove_filter( 'query', array( $this, 'fail_option_reads' ) );
+		$wpdb->suppress_errors( $suppress );
+
+		$this->assertNotNull( $result, 'A failed read must not read as a missing secret.' );
+		$this->assertWPError( $result );
+		$this->assertSame( WP_SECRETS_ERROR_STORE_UNAVAILABLE, $result->get_error_code() );
+	}
+
+	/**
+	 * @ticket 66424
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 */
+	public function test_get_returns_an_error_when_a_network_record_read_fails(): void {
+		global $wpdb;
+
+		$store = new WP_Secrets_Option_Store();
+		$store->set( 'myplugin/key', $this->sample_record(), true );
+		wp_cache_flush();
+
+		$suppress = $wpdb->suppress_errors( true );
+		add_filter( 'query', array( $this, 'fail_option_reads' ) );
+
+		$result = $store->get( 'myplugin/key', true );
+
+		remove_filter( 'query', array( $this, 'fail_option_reads' ) );
+		$wpdb->suppress_errors( $suppress );
+
+		$this->assertNotNull( $result, 'A failed read must not read as a missing secret.' );
+		$this->assertWPError( $result );
+		$this->assertSame( WP_SECRETS_ERROR_STORE_UNAVAILABLE, $result->get_error_code() );
+	}
+
+	/**
+	 * A read that never reaches the database returns from wpdb::query() before
+	 * wpdb::$last_error is set, so failure has to be read from the query result.
+	 *
+	 * @ticket 66424
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 */
+	public function test_get_returns_an_error_when_the_read_never_reaches_the_database(): void {
+		global $wpdb;
+
+		$store = new WP_Secrets_Option_Store();
+		$store->set( 'myplugin/key', $this->sample_record() );
+		wp_cache_delete( '_wp_secret_myplugin/key', 'options' );
+
+		$wpdb->last_error = '';
+		$wpdb->ready      = false;
+
+		$result = $store->get( 'myplugin/key' );
+
+		$wpdb->ready = true;
+
+		$this->assertNotNull( $result, 'A read that never reached the database must not read as a missing secret.' );
+		$this->assertWPError( $result );
+		$this->assertSame( WP_SECRETS_ERROR_STORE_UNAVAILABLE, $result->get_error_code() );
+	}
+
+	/**
+	 * wp_sitemeta.meta_value is nullable, unlike wp_options.option_value, so a row
+	 * can exist while holding no record. That is a malformed record, not absence.
+	 *
+	 * @ticket 66424
+	 *
+	 * @group ms-required
+	 *
+	 * @global wpdb $wpdb WordPress database abstraction object.
+	 */
+	public function test_get_returns_an_error_for_a_network_row_with_no_value(): void {
+		global $wpdb;
+
+		$wpdb->insert(
+			$wpdb->sitemeta,
+			array(
+				'site_id'    => get_current_network_id(),
+				'meta_key'   => '_wp_network_secret_myplugin/key',
+				'meta_value' => null,
+			)
+		);
+		wp_cache_flush();
+
+		$store = new WP_Secrets_Option_Store();
+
+		$result = $store->get( 'myplugin/key', true );
+
+		$this->assertNotNull( $result, 'A row that exists must not read as a missing secret.' );
+		$this->assertWPError( $result );
+		$this->assertSame( WP_SECRETS_ERROR_RECORD_MALFORMED, $result->get_error_code() );
+	}
+
+	/**
+	 * Makes every read of a stored option or network option fail.
+	 *
+	 * @param string $query The query about to run.
+	 * @return string The query to run instead.
+	 */
+	public function fail_option_reads( $query ) {
+		if ( 0 === stripos( $query, 'SELECT' ) && ( false !== stripos( $query, 'option_value' ) || false !== stripos( $query, 'meta_value' ) ) ) {
+			return 'SELECT this is not valid SQL';
+		}
+
+		return $query;
+	}
+
 	public function test_delete_removes_an_existing_record(): void {
 		$store = new WP_Secrets_Option_Store();
 		$store->set( 'myplugin/key', $this->sample_record() );