From 0c5c55f218096ae7ff556b6f62ee4c683494702b Mon Sep 17 00:00:00 2001 From: mpeshev Date: Sun, 25 Oct 2026 13:08:41 +0200 Subject: [PATCH 1/3] Secrets API: Report a broken or unreachable secrets backend in Site Health. Two Site Health tests reported a backend whose secrets cannot be listed as healthy: `wp_secrets_site_health_find_undecryptable()` and `wp_secrets_site_health_count_needing_rotation()` turned the `WP_Error` from `wp_list_secrets()` into an empty result, which read as "All secrets can be decrypted" and "No secrets are pending rotation". The key source test also detected a broken `secrets.php` drop-in by looking for "broken" in the keyring's description, which is a translated string, so on translated sites a broken drop-in was reported as using a dedicated key. Return the `WP_Error` from both helpers and report it, and detect the broken keyring by its class. Follow-up to [64337]. Co-Authored-By: Claude Opus 5.5 --- src/wp-admin/includes/secrets-site-health.php | 56 ++++++++++++++----- tests/phpunit/tests/secrets/siteHealth.php | 47 ++++++++++++++++ 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/src/wp-admin/includes/secrets-site-health.php b/src/wp-admin/includes/secrets-site-health.php index eef44e9266f18..3be93203d27d9 100644 --- a/src/wp-admin/includes/secrets-site-health.php +++ b/src/wp-admin/includes/secrets-site-health.php @@ -83,9 +83,11 @@ function wp_secrets_site_health_result( string $test, string $label, string $sta * @phpstan-return array{test: string, label: string, status: 'good'|'recommended'|'critical', badge: array{label: string, color: string}, description: string} */ function wp_secrets_site_health_test_key_source(): array { - $key_source = _wp_secrets_get_key_manager()->get_keyring()->get_key_source(); + $keyring = _wp_secrets_get_key_manager()->get_keyring(); + $key_source = $keyring->get_key_source(); - if ( str_contains( $key_source, 'broken' ) ) { + // The broken keyring's description is translated, so check its class rather than its text. + if ( $keyring instanceof WP_Secrets_Broken_Keyring ) { return wp_secrets_site_health_result( 'secrets_api_key_source', __( 'Secrets API key source is broken' ), @@ -144,8 +146,23 @@ function wp_secrets_site_health_test_key_source(): array { function wp_secrets_site_health_test_undecryptable(): array { $broken = wp_secrets_site_health_find_undecryptable( false ); - if ( is_multisite() && is_super_admin() ) { - $broken = array_merge( $broken, wp_secrets_site_health_find_undecryptable( true ) ); + if ( ! is_wp_error( $broken ) && is_multisite() && is_super_admin() ) { + $network_broken = wp_secrets_site_health_find_undecryptable( true ); + $broken = is_wp_error( $network_broken ) ? $network_broken : array_merge( $broken, $network_broken ); + } + + // Secrets that cannot be listed cannot be checked, which must not read as all of them decrypting. + if ( is_wp_error( $broken ) ) { + return wp_secrets_site_health_result( + 'secrets_api_undecryptable', + __( 'Secrets could not be checked' ), + 'critical', + sprintf( + '

%s

%s

', + esc_html__( 'The stored secrets could not be listed, so none of them could be checked. Secrets probably cannot be read or written either until this is fixed.' ), + esc_html( $broken->get_error_message() ) + ) + ); } if ( empty( $broken ) ) { @@ -190,15 +207,16 @@ function wp_secrets_site_health_test_undecryptable(): array { * @since 7.2.0 * * @param bool $network Whether to check network-scope secrets. - * @return array List of array( 'name' => ..., 'fingerprint' => ... ). + * @return array|WP_Error List of array( 'name' => ..., 'fingerprint' => ... ), + * or WP_Error when the secrets cannot be listed. * - * @phpstan-return list + * @phpstan-return list|WP_Error */ -function wp_secrets_site_health_find_undecryptable( bool $network ): array { +function wp_secrets_site_health_find_undecryptable( bool $network ) { $entries = $network ? wp_list_network_secrets() : wp_list_secrets(); if ( is_wp_error( $entries ) ) { - return array(); + return $entries; } $broken = array(); @@ -229,8 +247,18 @@ function wp_secrets_site_health_find_undecryptable( bool $network ): array { function wp_secrets_site_health_test_needs_rotation(): array { $count = wp_secrets_site_health_count_needing_rotation( false ); - if ( is_multisite() && is_super_admin() ) { - $count += wp_secrets_site_health_count_needing_rotation( true ); + if ( ! is_wp_error( $count ) && is_multisite() && is_super_admin() ) { + $network_count = wp_secrets_site_health_count_needing_rotation( true ); + $count = is_wp_error( $network_count ) ? $network_count : $count + $network_count; + } + + if ( is_wp_error( $count ) ) { + return wp_secrets_site_health_result( + 'secrets_api_needs_rotation', + __( 'Secrets could not be checked for pending rotation' ), + 'recommended', + sprintf( '

%s

', esc_html( $count->get_error_message() ) ) + ); } if ( 0 === $count ) { @@ -269,15 +297,15 @@ function wp_secrets_site_health_test_needs_rotation(): array { * @since 7.2.0 * * @param bool $network Whether to check network-scope secrets. - * @return int + * @return int|WP_Error The number of secrets, or WP_Error when the secrets cannot be listed. * - * @phpstan-return int<0, max> + * @phpstan-return int<0, max>|WP_Error */ -function wp_secrets_site_health_count_needing_rotation( bool $network ): int { +function wp_secrets_site_health_count_needing_rotation( bool $network ) { $entries = $network ? wp_list_network_secrets() : wp_list_secrets(); if ( is_wp_error( $entries ) ) { - return 0; + return $entries; } $count = 0; diff --git a/tests/phpunit/tests/secrets/siteHealth.php b/tests/phpunit/tests/secrets/siteHealth.php index d0eee49ceb421..56a3e4365fc82 100644 --- a/tests/phpunit/tests/secrets/siteHealth.php +++ b/tests/phpunit/tests/secrets/siteHealth.php @@ -98,6 +98,53 @@ public function test_key_source_test_is_critical_when_the_keyring_is_broken(): v $this->assertSame( 'critical', $result['status'] ); } + /** + * The broken keyring describes itself with a translated string. + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_key_source_test_is_critical_when_the_keyring_is_broken_on_a_translated_site(): void { + $GLOBALS['wp_secrets_dropin_broken'] = true; + + add_filter( + 'gettext', + static function ( $translation, $text ) { + return 'broken secrets.php drop-in' === $text ? 'defektes secrets.php-Drop-in' : $translation; + }, + 10, + 2 + ); + + $result = wp_secrets_site_health_test_key_source(); + + $this->assertSame( 'critical', $result['status'] ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_undecryptable_test_is_critical_when_the_secrets_cannot_be_listed(): void { + $GLOBALS['wp_secrets_dropin_broken'] = true; + + $result = wp_secrets_site_health_test_undecryptable(); + + $this->assertSame( 'critical', $result['status'] ); + } + + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_needs_rotation_test_is_recommended_when_the_secrets_cannot_be_listed(): void { + $GLOBALS['wp_secrets_dropin_broken'] = true; + + $result = wp_secrets_site_health_test_needs_rotation(); + + $this->assertSame( 'recommended', $result['status'] ); + } + public function test_undecryptable_test_is_good_when_nothing_is_broken(): void { wp_set_secret( 'myplugin/api-key', 'value' ); From e6b9ad79ac9625070c9b1a1143755f9e938cd3cc Mon Sep 17 00:00:00 2001 From: mpeshev Date: Sun, 25 Oct 2026 13:11:15 +0200 Subject: [PATCH 2/3] Secrets API: Do not recommend `WP_SECRETS_KEY` when a drop-in replaces the provider. A `secrets.php` drop-in can replace the whole secrets provider, as a host's KMS or vault integration does. Such a provider protects secrets with its own keys, but the key source Site Health test still described the unused default keyring, and recommended defining `WP_SECRETS_KEY`, which that provider never reads. Report the provider's label instead, with a good status. Follow-up to [64337]. Co-Authored-By: Claude Opus 5.5 --- src/wp-admin/includes/secrets-site-health.php | 24 +++++++++++++++++++ tests/phpunit/tests/secrets/siteHealth.php | 22 +++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/wp-admin/includes/secrets-site-health.php b/src/wp-admin/includes/secrets-site-health.php index 3be93203d27d9..a3283e1d96018 100644 --- a/src/wp-admin/includes/secrets-site-health.php +++ b/src/wp-admin/includes/secrets-site-health.php @@ -83,6 +83,30 @@ function wp_secrets_site_health_result( string $test, string $label, string $sta * @phpstan-return array{test: string, label: string, status: 'good'|'recommended'|'critical', badge: array{label: string, color: string}, description: string} */ function wp_secrets_site_health_test_key_source(): array { + $provider = _wp_secrets_get_provider(); + + /* + * A secrets.php drop-in can replace the whole provider, which then protects secrets + * with its own keys. The default keyring, and WP_SECRETS_KEY, are not used at all. + */ + if ( ! $provider instanceof WP_Secrets_Libsodium_Provider && ! $provider instanceof WP_Secrets_Broken_Provider ) { + return wp_secrets_site_health_result( + 'secrets_api_key_source', + __( 'Secrets API keys are managed by the provider' ), + 'good', + sprintf( + '

%s

', + esc_html( + sprintf( + /* translators: %s: Name of the active secrets provider. */ + __( 'Secrets are protected by the provider of the secrets.php drop-in: %s. WP_SECRETS_KEY is not used.' ), + $provider->get_label() + ) + ) + ) + ); + } + $keyring = _wp_secrets_get_key_manager()->get_keyring(); $key_source = $keyring->get_key_source(); diff --git a/tests/phpunit/tests/secrets/siteHealth.php b/tests/phpunit/tests/secrets/siteHealth.php index 56a3e4365fc82..32e8ea6cf3904 100644 --- a/tests/phpunit/tests/secrets/siteHealth.php +++ b/tests/phpunit/tests/secrets/siteHealth.php @@ -121,6 +121,28 @@ static function ( $translation, $text ) { $this->assertSame( 'critical', $result['status'] ); } + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + /** + * A provider from a secrets.php drop-in does not use the default keyring. + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_key_source_test_is_good_when_a_dropin_replaces_the_provider(): void { + $provider = $this->createMock( WP_Secrets_Provider::class ); + $provider->method( 'get_label' )->willReturn( 'Example Vault' ); + + $GLOBALS['wp_secrets_provider'] = $provider; + + $result = wp_secrets_site_health_test_key_source(); + + $this->assertSame( 'good', $result['status'] ); + $this->assertStringContainsString( 'Example Vault', $result['description'] ); + } + /** * @runInSeparateProcess * @preserveGlobalState disabled From 24e484e1eb76c141a30816f926ea0354472123a1 Mon Sep 17 00:00:00 2001 From: mpeshev Date: Sun, 25 Oct 2026 13:15:01 +0200 Subject: [PATCH 3/3] Secrets API: Stop Site Health advice that locks stored secrets. The key source Site Health test recommended defining `WP_SECRETS_KEY`, or regenerating it, "with a value from `wp secret generate-key`". That command is not part of core or WP-CLI. Following the advice after any secret was stored also makes every stored secret unreadable and blocks saving new ones: the root key stays wrapped with the previous site key, and core has no path that re-wraps it. The key source test then reported the new key as good. * Report a configured key that cannot unwrap the stored root key as critical, and say that restoring the previous value fixes it. * Recommend a dedicated key only before any secret is stored. Once secrets exist, warn that changing the key or the salts it derives from makes them unreadable. * Drop the reference to the WP-CLI command. * Say in the undecryptable secrets test that restoring the previous key makes the secrets readable again, instead of that there is no way to recover them. Follow-up to [64337]. Co-Authored-By: Claude Opus 5.5 --- src/wp-admin/includes/secrets-site-health.php | 39 ++++++++++++++++--- tests/phpunit/tests/secrets/siteHealth.php | 32 ++++++++++++++- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/wp-admin/includes/secrets-site-health.php b/src/wp-admin/includes/secrets-site-health.php index a3283e1d96018..e62ef4dda7de8 100644 --- a/src/wp-admin/includes/secrets-site-health.php +++ b/src/wp-admin/includes/secrets-site-health.php @@ -123,14 +123,41 @@ function wp_secrets_site_health_test_key_source(): array { ); } + /* + * Once a root key exists, it is wrapped with the current site key, and core has no way + * to re-wrap it under another one. Changing the key source then locks every stored + * secret, and blocks saving new ones, until the change is reverted. + */ + $has_root_key = false !== get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + + if ( $has_root_key ) { + $root_key = _wp_secrets_get_key_manager()->get_root_key(); + + if ( is_wp_error( $root_key ) ) { + return wp_secrets_site_health_result( + 'secrets_api_key_source', + __( 'Secrets API key does not match the stored secrets' ), + 'critical', + sprintf( + '

%s

', + esc_html__( 'The configured key cannot decrypt the root key that protects the stored secrets, so no secret can be read or saved. This happens when WP_SECRETS_KEY, LOGGED_IN_KEY, or LOGGED_IN_SALT changed after secrets were stored. Restoring the previous value fixes it.' ) + ) + ); + } + + wp_secrets_memzero( $root_key ); + } + if ( str_contains( $key_source, 'legacy interpretation' ) ) { return wp_secrets_site_health_result( 'secrets_api_key_source', - __( 'Secrets API key should be regenerated' ), + __( 'Secrets API key is not in the recommended form' ), 'recommended', sprintf( '

%s

', - esc_html__( 'WP_SECRETS_KEY is defined, but not in the recommended base64-encoded 32-byte form. Regenerate it with `wp secret generate-key`.' ) + $has_root_key + ? esc_html__( 'WP_SECRETS_KEY is defined, but not as a base64-encoded random 32-byte value. Keep its current value: secrets are already encrypted with it, and changing it makes them unreadable.' ) + : esc_html__( 'WP_SECRETS_KEY is defined, but not as a base64-encoded random 32-byte value. No secrets are stored yet, so it can still be replaced with such a value.' ) ) ); } @@ -142,7 +169,9 @@ function wp_secrets_site_health_test_key_source(): array { 'recommended', sprintf( '

%s

', - esc_html__( 'No dedicated WP_SECRETS_KEY constant is defined. Secrets are encrypted using a key derived from existing salts, which works but is not the recommended configuration. Define WP_SECRETS_KEY with a value from `wp secret generate-key`.' ) + $has_root_key + ? esc_html__( 'Secrets are encrypted with a key derived from the LOGGED_IN_KEY and LOGGED_IN_SALT constants. Changing either constant, or defining WP_SECRETS_KEY, makes every stored secret unreadable and prevents saving new ones until the change is reverted.' ) + : esc_html__( 'No dedicated WP_SECRETS_KEY constant is defined. Before any secrets are stored, define WP_SECRETS_KEY in wp-config.php as a base64-encoded random 32-byte value, so that secrets do not depend on the LOGGED_IN_KEY and LOGGED_IN_SALT constants.' ) ) ); } @@ -218,9 +247,9 @@ function wp_secrets_site_health_test_undecryptable(): array { 'critical', sprintf( '

%s

    %s

%s

', - esc_html__( 'The following secrets exist but could not be decrypted with the current key. There is no way to recover the original value. The credential must be re-entered.' ), + esc_html__( 'The following secrets exist but could not be decrypted with the current key.' ), $items, - esc_html__( 'This can happen after losing WP_SECRETS_KEY, restoring a database backup without its matching key, or a failed key rotation.' ) + esc_html__( 'This can happen after WP_SECRETS_KEY, LOGGED_IN_KEY, or LOGGED_IN_SALT changed, after restoring a database backup without its matching key, or after a failed key rotation. Restoring the previous key makes the secrets readable again. Otherwise, each credential must be entered again.' ) ) ); } diff --git a/tests/phpunit/tests/secrets/siteHealth.php b/tests/phpunit/tests/secrets/siteHealth.php index 32e8ea6cf3904..56a3d23d226f6 100644 --- a/tests/phpunit/tests/secrets/siteHealth.php +++ b/tests/phpunit/tests/secrets/siteHealth.php @@ -71,7 +71,37 @@ public function test_key_source_test_recommends_regeneration_for_a_legacy_key(): $result = wp_secrets_site_health_test_key_source(); $this->assertSame( 'recommended', $result['status'] ); - $this->assertStringContainsString( 'generate-key', $result['description'] ); + $this->assertStringContainsString( 'can still be replaced', $result['description'] ); + $this->assertStringNotContainsString( 'generate-key', $result['description'], 'There is no such WP-CLI command in core.' ); + } + + /** + * Replacing the key once secrets are stored would make them unreadable. + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_key_source_test_warns_to_keep_a_legacy_key_once_secrets_are_stored(): void { + define( 'WP_SECRETS_KEY', 'not-base64-32!!' ); + + $this->assertTrue( wp_set_secret( 'myplugin/api-key', 'value' ) ); + + $result = wp_secrets_site_health_test_key_source(); + + $this->assertSame( 'recommended', $result['status'] ); + $this->assertStringContainsString( 'Keep its current value', $result['description'] ); + } + + public function test_key_source_test_is_critical_when_the_key_does_not_match_the_stored_secrets(): void { + wp_set_secret( 'myplugin/api-key', 'value' ); + + // Stands in for a root key that was wrapped with a different site key. + update_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION, base64_encode( random_bytes( 72 ) ) ); + + $result = wp_secrets_site_health_test_key_source(); + + $this->assertSame( 'critical', $result['status'] ); + $this->assertStringContainsString( 'Restoring the previous value', $result['description'] ); } /**