From eadd369ee628f06b90ace2de0fd172bb24dbce73 Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Fri, 6 Nov 2026 08:52:55 -0800 Subject: [PATCH] Secrets API: Make the provider, store and keyring abstract classes. `WP_Secrets_Provider`, `WP_Secrets_Store` and `WP_Secrets_Keyring` were interfaces. Once 7.2 ships, adding a method to an interface is a fatal error in every drop-in that implements it, so they could never change. As abstract classes, a later release can add a method with a default implementation, and existing drop-ins keep working. Drop-ins now extend these classes. The method signatures are unchanged. The files are renamed from `interface-wp-secrets-*.php` to `class-wp-secrets-*.php`. See #66187. --- src/wp-includes/load.php | 12 +++---- .../class-wp-secrets-broken-keyring.php | 2 +- .../class-wp-secrets-broken-provider.php | 2 +- .../secrets/class-wp-secrets-broken-store.php | 2 +- .../class-wp-secrets-config-key-provider.php | 2 +- ...yring.php => class-wp-secrets-keyring.php} | 16 +++++---- .../class-wp-secrets-libsodium-provider.php | 4 +-- .../secrets/class-wp-secrets-option-store.php | 2 +- ...ider.php => class-wp-secrets-provider.php} | 34 +++++++++++-------- ...s-store.php => class-wp-secrets-store.php} | 16 +++++---- src/wp-settings.php | 6 ++-- .../includes/secrets/class-mock-keyring.php | 2 +- .../includes/secrets/class-mock-store.php | 2 +- .../class-wp-secrets-keyring-conformance.php | 2 +- .../class-wp-secrets-provider-conformance.php | 2 +- .../tests/secrets/brokenDropinFallbacks.php | 12 +++---- .../phpunit/tests/secrets/extensionPoints.php | 30 ++++++++++++++++ tests/phpunit/tests/secrets/provider.php | 2 +- .../secrets/readonlyProviderConformance.php | 2 +- .../secrets/wpSecretsConfigKeyProvider.php | 6 ++-- .../tests/secrets/wpSecretsKeyManager.php | 2 +- .../tests/secrets/wpSecretsOptionStore.php | 6 ++-- 22 files changed, 104 insertions(+), 62 deletions(-) rename src/wp-includes/secrets/{interface-wp-secrets-keyring.php => class-wp-secrets-keyring.php} (73%) rename src/wp-includes/secrets/{interface-wp-secrets-provider.php => class-wp-secrets-provider.php} (89%) rename src/wp-includes/secrets/{interface-wp-secrets-store.php => class-wp-secrets-store.php} (78%) diff --git a/src/wp-includes/load.php b/src/wp-includes/load.php index 31f82524bab85..1f42a45db10e7 100644 --- a/src/wp-includes/load.php +++ b/src/wp-includes/load.php @@ -828,8 +828,8 @@ function wp_using_ext_object_cache( $using = null ) { * Loads the secrets.php drop-in, if one exists, and records whether it left the * Secrets API's provider, store, and keyring overrides in a usable state. * - * Runs once, before must-use plugins load, so that every interface a drop-in's - * classes implement already exists and every plugin sees the drop-in's provider. + * Runs once, before must-use plugins load, so that every class a drop-in's classes + * extend already exists and every plugin sees the drop-in's provider. * * Sets `$GLOBALS['wp_secrets_dropin_loaded']` as soon as the file is found, whether * or not it then loads cleanly: wp_using_secrets_dropin() reports presence, not @@ -837,14 +837,14 @@ function wp_using_ext_object_cache( $using = null ) { * * Sets `$GLOBALS['wp_secrets_dropin_broken']` when the require throws, or when any of * `$GLOBALS['wp_secrets_provider']`, `$GLOBALS['wp_secrets_store']`, or - * `$GLOBALS['wp_secrets_keyring']` is set to something that does not implement its - * interface. A global left unset is fine: a drop-in that overrides only the keyring + * `$GLOBALS['wp_secrets_keyring']` is set to something that does not extend its + * class. A global left unset is fine: a drop-in that overrides only the keyring * leaves the other two alone. Once the flag is set, every Secrets API operation * returns WP_Error for the rest of the request rather than falling back to the * default provider, so a broken drop-in reads as unreachable, never as absent. * - * PHP treats a class that implements an interface but omits a required method as an - * uncatchable fatal error, even inside the try/catch around the require. That case + * PHP treats a class that extends an abstract class but omits one of its abstract + * methods as an uncatchable fatal error, even inside the try/catch around the require. That case * cannot be intercepted from PHP code. * * @since 7.2.0 diff --git a/src/wp-includes/secrets/class-wp-secrets-broken-keyring.php b/src/wp-includes/secrets/class-wp-secrets-broken-keyring.php index 85b51f9e05e67..adeba2bcbb580 100644 --- a/src/wp-includes/secrets/class-wp-secrets-broken-keyring.php +++ b/src/wp-includes/secrets/class-wp-secrets-broken-keyring.php @@ -20,7 +20,7 @@ * * @since 7.2.0 */ -final class WP_Secrets_Broken_Keyring implements WP_Secrets_Keyring { +final class WP_Secrets_Broken_Keyring extends WP_Secrets_Keyring { /** * Always fails closed. diff --git a/src/wp-includes/secrets/class-wp-secrets-broken-provider.php b/src/wp-includes/secrets/class-wp-secrets-broken-provider.php index 8c429c55e0705..df84034299932 100644 --- a/src/wp-includes/secrets/class-wp-secrets-broken-provider.php +++ b/src/wp-includes/secrets/class-wp-secrets-broken-provider.php @@ -22,7 +22,7 @@ * * @since 7.2.0 */ -final class WP_Secrets_Broken_Provider implements WP_Secrets_Provider { +final class WP_Secrets_Broken_Provider extends WP_Secrets_Provider { /** * The single error every operation reports. diff --git a/src/wp-includes/secrets/class-wp-secrets-broken-store.php b/src/wp-includes/secrets/class-wp-secrets-broken-store.php index bdb24327f3021..affa2d7bf1da6 100644 --- a/src/wp-includes/secrets/class-wp-secrets-broken-store.php +++ b/src/wp-includes/secrets/class-wp-secrets-broken-store.php @@ -21,7 +21,7 @@ * * @since 7.2.0 */ -final class WP_Secrets_Broken_Store implements WP_Secrets_Store { +final class WP_Secrets_Broken_Store extends WP_Secrets_Store { /** * Always fails closed. diff --git a/src/wp-includes/secrets/class-wp-secrets-config-key-provider.php b/src/wp-includes/secrets/class-wp-secrets-config-key-provider.php index c99adc925ecd0..0744b4d9a8020 100644 --- a/src/wp-includes/secrets/class-wp-secrets-config-key-provider.php +++ b/src/wp-includes/secrets/class-wp-secrets-config-key-provider.php @@ -32,7 +32,7 @@ * * @since 7.2.0 */ -final class WP_Secrets_Config_Key_Provider implements WP_Secrets_Keyring { +final class WP_Secrets_Config_Key_Provider extends WP_Secrets_Keyring { /** * AAD binding the root key's wrapped form to this purpose. diff --git a/src/wp-includes/secrets/interface-wp-secrets-keyring.php b/src/wp-includes/secrets/class-wp-secrets-keyring.php similarity index 73% rename from src/wp-includes/secrets/interface-wp-secrets-keyring.php rename to src/wp-includes/secrets/class-wp-secrets-keyring.php index d378062908a9e..57ea160f9fad9 100644 --- a/src/wp-includes/secrets/interface-wp-secrets-keyring.php +++ b/src/wp-includes/secrets/class-wp-secrets-keyring.php @@ -1,6 +1,6 @@ |WP_Error */ - public function list_secrets( string $name_prefix = '', bool $network = false ); + abstract public function list_secrets( string $name_prefix = '', bool $network = false ); /** * A short human-readable name for this provider, for Site Health. @@ -201,7 +205,7 @@ public function list_secrets( string $name_prefix = '', bool $network = false ); * * @return string */ - public function get_label(): string; + abstract public function get_label(): string; /** * Where secrets this provider holds are actually protected. @@ -218,7 +222,7 @@ public function get_label(): string; * * @phpstan-return self::BOUNDARY_* */ - public function get_protection_boundary(): string; + abstract public function get_protection_boundary(): string; /** * Whether wp_set_secret() can succeed against this provider. @@ -232,5 +236,5 @@ public function get_protection_boundary(): string; * * @return bool */ - public function is_writable(): bool; + abstract public function is_writable(): bool; } diff --git a/src/wp-includes/secrets/interface-wp-secrets-store.php b/src/wp-includes/secrets/class-wp-secrets-store.php similarity index 78% rename from src/wp-includes/secrets/interface-wp-secrets-store.php rename to src/wp-includes/secrets/class-wp-secrets-store.php index 81759a483bec4..4431eacc144c2 100644 --- a/src/wp-includes/secrets/interface-wp-secrets-store.php +++ b/src/wp-includes/secrets/class-wp-secrets-store.php @@ -1,6 +1,6 @@ |null|WP_Error The record array if it exists. Null if it does not. * WP_Error if it could not be determined which. */ - public function get( string $name, bool $network = false ); + abstract public function get( string $name, bool $network = false ); /** * Writes a secret's record. @@ -47,7 +51,7 @@ public function get( string $name, bool $network = false ); * @return bool|WP_Error True on success. WP_Error on failure, including when the * store does not accept writes. */ - public function set( string $name, array $record, bool $network = false ); + abstract public function set( string $name, array $record, bool $network = false ); /** * Deletes a secret's record. @@ -59,7 +63,7 @@ public function set( string $name, array $record, bool $network = false ); * @return bool|WP_Error True on success (including if it did not exist). * WP_Error on failure. */ - public function delete( string $name, bool $network = false ); + abstract public function delete( string $name, bool $network = false ); /** * Lists the names of every secret in this store, for this scope. Never a value. @@ -69,5 +73,5 @@ public function delete( string $name, bool $network = false ); * @param bool $network Whether to list network-scope secrets. * @return string[]|WP_Error Array of secret names on success. WP_Error on failure. */ - public function list_names( bool $network = false ); + abstract public function list_names( bool $network = false ); } diff --git a/src/wp-settings.php b/src/wp-settings.php index 92f9456aa02ca..3c5bc8542eaa8 100644 --- a/src/wp-settings.php +++ b/src/wp-settings.php @@ -324,9 +324,9 @@ require ABSPATH . WPINC . '/secrets.php'; require ABSPATH . WPINC . '/secrets/class-wp-secret-version.php'; require ABSPATH . WPINC . '/secrets/class-wp-secret.php'; -require ABSPATH . WPINC . '/secrets/interface-wp-secrets-provider.php'; -require ABSPATH . WPINC . '/secrets/interface-wp-secrets-keyring.php'; -require ABSPATH . WPINC . '/secrets/interface-wp-secrets-store.php'; +require ABSPATH . WPINC . '/secrets/class-wp-secrets-provider.php'; +require ABSPATH . WPINC . '/secrets/class-wp-secrets-keyring.php'; +require ABSPATH . WPINC . '/secrets/class-wp-secrets-store.php'; require ABSPATH . WPINC . '/secrets/class-wp-secrets-config-key-provider.php'; require ABSPATH . WPINC . '/secrets/class-wp-secrets-broken-keyring.php'; require ABSPATH . WPINC . '/secrets/class-wp-secrets-cipher.php'; diff --git a/tests/phpunit/includes/secrets/class-mock-keyring.php b/tests/phpunit/includes/secrets/class-mock-keyring.php index 1272cad4d1a1f..a32788ea7e174 100644 --- a/tests/phpunit/includes/secrets/class-mock-keyring.php +++ b/tests/phpunit/includes/secrets/class-mock-keyring.php @@ -5,7 +5,7 @@ * a real keyring under the conformance suite without needing that code to also * exercise libsodium. */ -class Mock_Keyring implements WP_Secrets_Keyring { +class Mock_Keyring extends WP_Secrets_Keyring { const MARKER = 'mock-wrapped:'; diff --git a/tests/phpunit/includes/secrets/class-mock-store.php b/tests/phpunit/includes/secrets/class-mock-store.php index 784a1db0280d0..bd621becb5b1b 100644 --- a/tests/phpunit/includes/secrets/class-mock-store.php +++ b/tests/phpunit/includes/secrets/class-mock-store.php @@ -5,7 +5,7 @@ * Can be told to fail any operation on demand, and records every value it is ever * handed so that a test can assert it was never given a plaintext. */ -class Mock_Store implements WP_Secrets_Store { +class Mock_Store extends WP_Secrets_Store { /** * Stored records, keyed by scope and name. diff --git a/tests/phpunit/includes/secrets/class-wp-secrets-keyring-conformance.php b/tests/phpunit/includes/secrets/class-wp-secrets-keyring-conformance.php index be4a87af4aca2..57ec3c4f9e447 100644 --- a/tests/phpunit/includes/secrets/class-wp-secrets-keyring-conformance.php +++ b/tests/phpunit/includes/secrets/class-wp-secrets-keyring-conformance.php @@ -3,7 +3,7 @@ * Conformance suite for WP_Secrets_Keyring implementations. * * Extend this, return a keyring from keyring(), and the contract every keyring is - * held to gets checked against it. "implements WP_Secrets_Keyring" is a claim PHP + * held to gets checked against it. "extends WP_Secrets_Keyring" is a claim PHP * can verify about method names and nothing else – it cannot tell you that * wrap() is non-deterministic, that unwrap() rejects tampering instead of * silently returning garbage bytes, or that failure is reported as WP_Error diff --git a/tests/phpunit/includes/secrets/class-wp-secrets-provider-conformance.php b/tests/phpunit/includes/secrets/class-wp-secrets-provider-conformance.php index 8e09a6ebdfb3b..10879e2dedd24 100644 --- a/tests/phpunit/includes/secrets/class-wp-secrets-provider-conformance.php +++ b/tests/phpunit/includes/secrets/class-wp-secrets-provider-conformance.php @@ -3,7 +3,7 @@ * Conformance suite for WP_Secrets_Provider implementations. * * Extend this, return a provider from provider(), and the contract every provider - * is held to gets checked against it. The point is that "implements + * is held to gets checked against it. The point is that "extends * WP_Secrets_Provider" is a claim PHP can verify about method names and nothing * else – it cannot tell you that absence is reported as null rather than false, or * that an unreachable backend fails closed instead of looking empty, and those are diff --git a/tests/phpunit/tests/secrets/brokenDropinFallbacks.php b/tests/phpunit/tests/secrets/brokenDropinFallbacks.php index 0d49cfc568c80..bdc96a06b7abb 100644 --- a/tests/phpunit/tests/secrets/brokenDropinFallbacks.php +++ b/tests/phpunit/tests/secrets/brokenDropinFallbacks.php @@ -11,12 +11,12 @@ class Tests_Secrets_BrokenDropinFallbacks extends WP_UnitTestCase { /** - * Ensure that WP_Secrets_Broken_Store implements the WP_Secrets_Store interface. + * Ensure that WP_Secrets_Broken_Store extends WP_Secrets_Store. * * @coversNothing */ - public function test_broken_store_implements_the_interface(): void { - $this->assertInstanceOf( WP_Secrets_Store::class, new WP_Secrets_Broken_Store(), 'WP_Secrets_Broken_Store should implement WP_Secrets_Store.' ); // @phpstan-ignore method.alreadyNarrowedType (Intentionally checking the class against its interface at runtime.) + public function test_broken_store_extends_the_class(): void { + $this->assertInstanceOf( WP_Secrets_Store::class, new WP_Secrets_Broken_Store(), 'WP_Secrets_Broken_Store should extend WP_Secrets_Store.' ); // @phpstan-ignore method.alreadyNarrowedType (Intentionally checking the class against its parent at runtime.) } /** @@ -68,12 +68,12 @@ public function test_broken_store_list_names_fails(): void { } /** - * Ensure that WP_Secrets_Broken_Keyring implements the WP_Secrets_Keyring interface. + * Ensure that WP_Secrets_Broken_Keyring extends WP_Secrets_Keyring. * * @coversNothing */ - public function test_broken_keyring_implements_the_interface(): void { - $this->assertInstanceOf( WP_Secrets_Keyring::class, new WP_Secrets_Broken_Keyring(), 'WP_Secrets_Broken_Keyring should implement WP_Secrets_Keyring.' ); // @phpstan-ignore method.alreadyNarrowedType (Intentionally checking the class against its interface at runtime.) + public function test_broken_keyring_extends_the_class(): void { + $this->assertInstanceOf( WP_Secrets_Keyring::class, new WP_Secrets_Broken_Keyring(), 'WP_Secrets_Broken_Keyring should extend WP_Secrets_Keyring.' ); // @phpstan-ignore method.alreadyNarrowedType (Intentionally checking the class against its parent at runtime.) } /** diff --git a/tests/phpunit/tests/secrets/extensionPoints.php b/tests/phpunit/tests/secrets/extensionPoints.php index 9d532ef2c95a4..88ec29e9b1bd1 100644 --- a/tests/phpunit/tests/secrets/extensionPoints.php +++ b/tests/phpunit/tests/secrets/extensionPoints.php @@ -253,4 +253,34 @@ public function test_a_broken_dropin_state_fails_every_operation_closed(): void $this->assertWPError( wp_retire_secret_version( 'myplugin/api-key' ), 'wp_retire_secret_version() should return a WP_Error when the drop-in is broken.' ); $this->assertFalse( wp_secrets_provider_is_writable(), 'The provider should not report as writable when the drop-in is broken.' ); } + + /** + * Ensure that the provider, store and keyring extension points are abstract classes. + * + * They are abstract classes rather than interfaces so a later release can add a + * method with a default implementation without breaking existing drop-ins. + * + * @coversNothing + * + * @dataProvider data_extension_points_are_abstract_classes + * + * @param string $class_name Name of the extension point. + */ + public function test_extension_points_are_abstract_classes( string $class_name ): void { + $reflection = new ReflectionClass( $class_name ); + + $this->assertFalse( $reflection->isInterface(), "{$class_name} should not be an interface." ); + $this->assertTrue( $reflection->isAbstract(), "{$class_name} should be an abstract class." ); + } + + /** + * @return array + */ + public static function data_extension_points_are_abstract_classes(): array { + return array( + 'provider' => array( 'WP_Secrets_Provider' ), + 'store' => array( 'WP_Secrets_Store' ), + 'keyring' => array( 'WP_Secrets_Keyring' ), + ); + } } diff --git a/tests/phpunit/tests/secrets/provider.php b/tests/phpunit/tests/secrets/provider.php index febea51d3b580..43511b2bb6aa6 100644 --- a/tests/phpunit/tests/secrets/provider.php +++ b/tests/phpunit/tests/secrets/provider.php @@ -16,7 +16,7 @@ class Tests_Secrets_Provider extends WP_UnitTestCase { * @return WP_Secrets_Provider */ private function read_only_provider() { - return new class() implements WP_Secrets_Provider { + return new class() extends WP_Secrets_Provider { /** * @param string $name The secret's name. * @param string $version A WP_Secret_Version constant. diff --git a/tests/phpunit/tests/secrets/readonlyProviderConformance.php b/tests/phpunit/tests/secrets/readonlyProviderConformance.php index f64b6f3dd7b32..3aa23e7aa9911 100644 --- a/tests/phpunit/tests/secrets/readonlyProviderConformance.php +++ b/tests/phpunit/tests/secrets/readonlyProviderConformance.php @@ -21,7 +21,7 @@ class Tests_Secrets_ReadOnlyProviderConformance extends WP_Secrets_Provider_Conf * @return WP_Secrets_Provider */ protected function provider() { - return new class() implements WP_Secrets_Provider { + return new class() extends WP_Secrets_Provider { /** * @param string $name The secret's name. * @param string $version A WP_Secret_Version constant. diff --git a/tests/phpunit/tests/secrets/wpSecretsConfigKeyProvider.php b/tests/phpunit/tests/secrets/wpSecretsConfigKeyProvider.php index bbc96a8f4a99c..724fbdd303aed 100644 --- a/tests/phpunit/tests/secrets/wpSecretsConfigKeyProvider.php +++ b/tests/phpunit/tests/secrets/wpSecretsConfigKeyProvider.php @@ -33,12 +33,12 @@ private function invoke_private( object $instance, string $method, array $args = } /** - * Ensure that WP_Secrets_Config_Key_Provider implements WP_Secrets_Keyring. + * Ensure that WP_Secrets_Config_Key_Provider extends WP_Secrets_Keyring. * * @coversNothing */ - public function test_implements_the_keyring_interface(): void { - $this->assertInstanceOf( WP_Secrets_Keyring::class, new WP_Secrets_Config_Key_Provider(), 'WP_Secrets_Config_Key_Provider should implement WP_Secrets_Keyring.' ); // @phpstan-ignore method.alreadyNarrowedType (Pins the interface the class declares, so removing it fails a test.) + public function test_extends_the_keyring_class(): void { + $this->assertInstanceOf( WP_Secrets_Keyring::class, new WP_Secrets_Config_Key_Provider(), 'WP_Secrets_Config_Key_Provider should extend WP_Secrets_Keyring.' ); // @phpstan-ignore method.alreadyNarrowedType (Pins the class it extends, so removing it fails a test.) } /** diff --git a/tests/phpunit/tests/secrets/wpSecretsKeyManager.php b/tests/phpunit/tests/secrets/wpSecretsKeyManager.php index b6bf4d0d1391a..044613cd0a9b9 100644 --- a/tests/phpunit/tests/secrets/wpSecretsKeyManager.php +++ b/tests/phpunit/tests/secrets/wpSecretsKeyManager.php @@ -361,7 +361,7 @@ public function test_rotate_fails_when_the_old_keyring_is_wrong(): void { $manager = new WP_Secrets_Key_Manager( $right_keyring ); $manager->get_root_key(); - $wrong_keyring = new class() implements WP_Secrets_Keyring { + $wrong_keyring = new class() extends WP_Secrets_Keyring { /** * @param string $key_material Raw key material. * @return string diff --git a/tests/phpunit/tests/secrets/wpSecretsOptionStore.php b/tests/phpunit/tests/secrets/wpSecretsOptionStore.php index 01c472e92b20a..5320b5c551efd 100644 --- a/tests/phpunit/tests/secrets/wpSecretsOptionStore.php +++ b/tests/phpunit/tests/secrets/wpSecretsOptionStore.php @@ -28,12 +28,12 @@ private function sample_record(): array { } /** - * Ensure that WP_Secrets_Option_Store implements the WP_Secrets_Store interface. + * Ensure that WP_Secrets_Option_Store extends WP_Secrets_Store. * * @coversNothing */ - public function test_implements_the_store_interface(): void { - $this->assertInstanceOf( WP_Secrets_Store::class, new WP_Secrets_Option_Store(), 'WP_Secrets_Option_Store should implement WP_Secrets_Store.' ); // @phpstan-ignore method.alreadyNarrowedType (Pins the interface the class declares, so removing it fails a test.) + public function test_extends_the_store_class(): void { + $this->assertInstanceOf( WP_Secrets_Store::class, new WP_Secrets_Option_Store(), 'WP_Secrets_Option_Store should extend WP_Secrets_Store.' ); // @phpstan-ignore method.alreadyNarrowedType (Pins the class it extends, so removing it fails a test.) } /**