Sync with core trunk through r64690, require WP 6.8, and fix the trunk CI legs

.github/workflows/ci.yml

@@ -100,6 +100,12 @@ jobs:
       matrix:
         php: ['7.4', '8.0', '8.3', '8.5']
         wp: ['latest', 'trunk']
+        # The declared floor, `Requires at least` in secrets-api.php. src/ relies on
+        # core's polyfills (array_all() arrived in 6.8), so the floor gets a leg of
+        # its own rather than being assumed from latest.
+        include:
+          - php: '7.4'
+            wp: '6.8'
     services:
       mysql:
         image: mysql:8.0

README.md

@@ -3,7 +3,7 @@
 Feature plugin for the [Secrets API proposed for WordPress 7.2][proposal]. Encrypted, versioned
 credential storage with pluggable storage and keyring back ends.
 
-This exists so contributors can *run* the API instead of reading about it, and so sites on 6.6
+This exists so contributors can *run* the API instead of reading about it, and so sites on 6.8
 and later get something usable before the core patch lands. Everything under `src/` is written to
 be copied verbatim into `wordpress-develop` — same paths, same coding style, same `default` text
 domain.
@@ -105,7 +105,7 @@ future screen needs are in scope; the screen is not.
 ## Requirements
 
 - PHP 7.4+ (core's floor — `src/` contains no PHP 8 syntax)
-- WordPress 6.6+
+- WordPress 6.8+ (core polyfills `array_all()` from 6.8, and `src/` uses it)
 - libsodium, via the extension or core's bundled `sodium_compat`
 
 ## Coexisting with the Displace prototype

docs/spec/extension-points.md

@@ -56,7 +56,7 @@ class Tests_My_Platform_Provider extends WP_Secrets_Provider_Conformance {
 It checks that a name never set reads as `null`; that `PREVIOUS` with no previous value is an
 absence rather than an error; that deleting or retiring something absent succeeds; that a stored
 value round-trips and a deleted one reads as absent; that fingerprints stay stable for the same
-value; that listings never contain a plaintext and respect a name prefix; that the label,
+value; that writing an unchanged value keeps the previous version; that listings never contain a plaintext and respect a name prefix; that the label,
 boundary, and writability declarations are well-formed; and that a provider declaring itself
 read-only really does refuse writes with `secret_provider_read_only`. Where the contract allows
 variation it adapts: a read-only provider is never asked to round-trip a value, and the skipped

docs/spec/rotation.md

@@ -24,7 +24,9 @@ the new value into `current`, and moves the old `current` to `previous` through
 which decrypts under the `current` binding and re-encrypts under `previous`. If the outgoing slot
 cannot be decrypted, it is dropped and the write proceeds, so a corrupted record never blocks its
 own repair. The `wp_secret_changed` action fires with `updated`. During the drain, `wp_get_secret(
-$name, WP_Secret_Version::PREVIOUS )` returns the old value.
+$name, WP_Secret_Version::PREVIOUS )` returns the old value. Saving the value `current` already
+holds is not a rotation: nothing is written and nothing fires, so a settings form that re-saves an
+unchanged key does not end the drain early. See [versioning.md](versioning.md).
 
 **Retiring the previous value.** `wp_retire_secret_version()` and
 `wp_retire_network_secret_version()` in `src/wp-includes/secrets.php` call the provider's

docs/spec/scope.md

@@ -15,7 +15,7 @@ scope", and "Timeline" in the [proposal][proposal].
 
 ## As built
 
-**The plugin.** `secrets-api.php` declares version `0.3.0`, `Requires at least: 6.6`, and
+**The plugin.** `secrets-api.php` declares version `0.3.0`, `Requires at least: 6.8`, and
 `Requires PHP: 7.4`.
 
 **Core-bound versus plugin-only.** Everything under `src/` is written to be copied into
@@ -38,7 +38,8 @@ It also brings the tests for everything in `src/`.
 
 **Standing down.** `wp_secrets_api_bootstrap()` in `secrets-api.php` loads nothing and shows an
 admin notice when `$wp_version` is at least `WP_SECRETS_API_CORE_VERSION` (`7.2`, overridable)
-**and** `wp_get_secret()` already exists. If the symbol exists but the version test fails, the
+**and** `wp_get_secret()` already exists. A beta, RC, or trunk build counts as its release, so
+`7.2-beta1-64606-src` passes the version test. If the symbol exists but the version test fails, the
 plugin refuses to load and shows a conflict notice instead of deferring to an unknown
 implementation. `tests/phpunit/test-secrets-noop-gate.php` covers the gate.
 

docs/spec/versioning.md

@@ -28,6 +28,13 @@ checks the same set before any cryptographic operation.
 and demotes the outgoing `current` to `previous`. Whatever was already in `previous` is not carried
 forward, so a third write discards the oldest value.
 
+**An unchanged write is not a rotation.** Writing the value `current` already holds, as a settings
+form does on every save, leaves the record untouched and does not fire `wp_secret_changed`, so
+`previous` survives. The one exception is a write that raises `needs_rotation` on a value not yet
+flagged, such as `wp_import_option_as_secret()` on an option holding the current value. It
+records the flag and carries `previous` over as stored. The `WP_Secrets_Provider::set()` contract
+requires any provider that keeps a previous version to keep it through an unchanged write.
+
 **Slot is part of the ciphertext binding.** The slot name is one of the fields in the AAD
 `WP_Secrets_Cipher::build_aad()` computes, so a slot's ciphertext is bound to its position.
 Demotion is therefore a decrypt-and-re-encrypt in

examples/README.md

@@ -84,7 +84,8 @@ class Tests_My_Platform_Provider extends WP_Secrets_Provider_Conformance {
 ```
 
 It covers what `implements WP_Secrets_Provider` can't: absence reported as `null`, deleting
-something absent succeeding, fingerprints staying stable for the same value, listings never
+something absent succeeding, fingerprints staying stable for the same value, an unchanged write
+keeping the previous version, listings never
 containing a plaintext, and a read-only declaration actually being honoured. See
 [`../docs/spec/extension-points.md`](../docs/spec/extension-points.md).
 

examples/aws-secrets-manager/secrets.php

@@ -138,6 +138,14 @@ public function get( string $name, string $version, bool $network = false ) {
 	public function set( string $name, $value, bool $network = false, bool $needs_rotation = false, ?string $action = null ) {
 		$aws_name = $this->aws_name( $name, $network );
 
+		// PutSecretValue makes a new version even for the value AWSCURRENT
+		// already holds, which would move AWSPREVIOUS onto that same value and
+		// lose the real previous one. An unchanged write is skipped instead –
+		// see WP_Secrets_Provider::set().
+		if ( $this->holds_current_value( $aws_name, $value ) ) {
+			return true;
+		}
+
 		// PutSecretValue rotates AWSCURRENT -> AWSPREVIOUS for us, which is the
 		// whole two-slot model handled server-side.
 		$result = $this->call(
@@ -278,6 +286,32 @@ public function list_secrets( string $name_prefix = '', bool $network = false )
 		return $entries;
 	}
 
+	/**
+	 * Whether AWSCURRENT already holds this value. A failed read is not an
+	 * answer, so it reports false and leaves the write to proceed and report
+	 * its own error.
+	 *
+	 * @param string $aws_name Name in Secrets Manager.
+	 * @param mixed  $value    Plaintext about to be written.
+	 *
+	 * @return bool
+	 */
+	private function holds_current_value( $aws_name, $value ) {
+		$response = $this->call(
+			'GetSecretValue',
+			array(
+				'SecretId'     => $aws_name,
+				'VersionStage' => 'AWSCURRENT',
+			)
+		);
+
+		return ! is_wp_error( $response )
+			&& isset( $response['SecretString'] )
+			&& is_string( $response['SecretString'] )
+			&& is_string( $value )
+			&& hash_equals( $response['SecretString'], $value );
+	}
+
 	/**
 	 * Whether a ListSecrets entry has a version labelled AWSPREVIOUS.
 	 *

examples/vault-provider/secrets.php

@@ -169,6 +169,42 @@ public function set( string $name, $value, bool $network = false, bool $needs_ro
 
 		$created = ( null === $meta );
 
+		// Writing the value Vault already holds as current would add version
+		// N+1 and push the real previous value out, so it is skipped – see
+		// WP_Secrets_Provider::set(). Raising the rotation flag on that value
+		// is still recorded, as a metadata write with no new version.
+		if ( ! $created && $this->holds_current_value( $vault_path, $value ) ) {
+			if ( ! $needs_rotation || $this->flag_is_set( $meta ) ) {
+				return true;
+			}
+
+			$flag = $this->write_flag( $vault_path, true, $meta );
+
+			if ( is_wp_error( $flag ) ) {
+				return new WP_Error(
+					WP_SECRETS_ERROR_STORE_UNAVAILABLE,
+					sprintf(
+						'Vault refused to record the rotation flag: %s',
+						$flag->get_error_message()
+					)
+				);
+			}
+
+			/** This action is documented in WP_Secrets_Libsodium_Provider::set(). */
+			do_action(
+				'wp_secret_changed',
+				$name,
+				null !== $action ? $action : 'updated',
+				get_current_user_id(),
+				time(),
+				'',
+				'',
+				$network
+			);
+
+			return true;
+		}
+
 		if ( $created ) {
 			$result = $this->request(
 				'POST',
@@ -599,6 +635,26 @@ private function flag_is_set( $meta ) {
 			&& '1' === $meta['custom_metadata'][ self::ROTATION_FLAG ];
 	}
 
+	/**
+	 * Whether the current version already holds this value. A failed read is
+	 * not an answer, so it reports false and leaves the write to proceed and
+	 * report its own error.
+	 *
+	 * @param string $vault_path Path under the mount.
+	 * @param mixed  $value      Plaintext about to be written.
+	 *
+	 * @return bool
+	 */
+	private function holds_current_value( $vault_path, $value ) {
+		$data = $this->request( 'GET', $this->url( 'data', $vault_path ) );
+
+		return is_array( $data )
+			&& isset( $data['data']['value'] )
+			&& is_string( $data['data']['value'] )
+			&& is_string( $value )
+			&& hash_equals( $data['data']['value'], $value );
+	}
+
 	/**
 	 * Writes the rotation flag. Vault replaces custom_metadata wholesale on
 	 * every POST, so this merges the flag into the custom_metadata already

examples/vault-provider/tests/test-vault-provider.php

@@ -185,6 +185,64 @@ public function test_only_two_versions_are_kept_in_vault_itself() {
 		$this->assertSame( 200, $this->server->read_version( $path, 3 ) );
 	}
 
+	public function test_an_unchanged_write_adds_no_version_and_fires_nothing() {
+		$path = 'wp/site/1/acme/key';
+
+		$this->provider->set( 'acme/key', 'v1' );
+		$this->provider->set( 'acme/key', 'v2' );
+
+		$fired = array();
+		add_action(
+			'wp_secret_changed',
+			static function ( $name, $action ) use ( &$fired ) {
+				$fired[] = $action;
+			},
+			10,
+			2
+		);
+
+		$this->assertTrue( $this->provider->set( 'acme/key', 'v2' ) );
+
+		$this->assertSame( 2, $this->server->metadata( $path )['current_version'] );
+		$this->assertSame( 'v1', $this->provider->get( 'acme/key', WP_Secret_Version::PREVIOUS )->reveal() );
+		$this->assertSame( array(), $fired );
+	}
+
+	public function test_flagging_an_unchanged_value_writes_the_flag_without_a_new_version() {
+		$path = 'wp/site/1/acme/key';
+
+		$this->provider->set( 'acme/key', 'v1' );
+		$this->provider->set( 'acme/key', 'v2' );
+
+		$fired = array();
+		add_action(
+			'wp_secret_changed',
+			static function ( $name, $action ) use ( &$fired ) {
+				$fired[] = $action;
+			},
+			10,
+			2
+		);
+
+		$this->assertTrue( $this->provider->set( 'acme/key', 'v2', false, true, 'imported' ) );
+
+		$meta = $this->server->metadata( $path );
+
+		$this->assertSame( 2, $meta['current_version'] );
+		$this->assertSame( '1', $meta['custom_metadata']['needs_rotation'] );
+		$this->assertSame( 'v1', $this->provider->get( 'acme/key', WP_Secret_Version::PREVIOUS )->reveal() );
+		$this->assertSame( array( 'imported' ), $fired );
+	}
+
+	public function test_an_unchanged_write_keeps_the_rotation_flag() {
+		$path = 'wp/site/1/acme/key';
+
+		$this->provider->set( 'acme/key', 'v1', false, true );
+		$this->provider->set( 'acme/key', 'v1' );
+
+		$this->assertSame( '1', $this->server->metadata( $path )['custom_metadata']['needs_rotation'] );
+	}
+
 	public function test_previous_is_strictly_n_minus_1_even_when_older_versions_survive() {
 		$path = 'wp/site/1/acme/key';
 

phpstan.neon.dist

@@ -24,6 +24,7 @@ parameters:
 	scanFiles:
 		- vendor/php-stubs/wp-cli-stubs/wp-cli-stubs.php
 		- vendor/php-stubs/wp-cli-stubs/wp-cli-commands-stubs.php
+		- tests/phpstan/compat-stubs.php
 	bootstrapFiles:
 		- tests/phpstan-bootstrap.php
 	treatPhpDocTypesAsCertain: false
@@ -37,3 +38,12 @@ parameters:
 			paths:
 				- src/wp-includes/secrets/class-wp-secrets-option-store.php
 				- plugin/class-secrets-api-legacy-reader.php
+		# r64674's no-op check reads $existing['current'] behind $is_unchanged, which is
+		# only true when $has_current is. PHPStan narrows $existing through $has_current
+		# but not through $is_unchanged. src/ stays as core committed it, so the one
+		# line is ignored here rather than rewritten.
+		-
+			message: '#^Cannot access offset ''needs_rotation'' on mixed\.$#'
+			identifier: offsetAccess.nonOffsetAccessible
+			path: src/wp-includes/secrets/class-wp-secrets-libsodium-provider.php
+			count: 1

secrets-api.php

@@ -4,7 +4,7 @@
  * Plugin URI:        https://github.com/ericmann/secrets-api
  * Description:       Feature plugin for the WordPress Secrets API proposed for 7.2. Encrypted, versioned credential storage with pluggable storage and keyring back ends.
  * Version:           0.3.0
- * Requires at least: 6.6
+ * Requires at least: 6.8
  * Requires PHP:      7.4
  * License:           GPL-2.0-or-later
  * License URI:       https://www.gnu.org/licenses/gpl-2.0.html
@@ -63,8 +63,12 @@ function wp_secrets_api_bootstrap(): void {
 
 	$symbol_taken = function_exists( 'wp_get_secret' );
 
+	// A beta, RC, or trunk build such as '7.2-beta1-64606-src' counts as its release:
+	// version_compare() alone ranks every pre-release below '7.2'.
+	$release = explode( '-', $wp_version )[0];
+
 	// Core ships the API. Load nothing; the plugin is redundant.
-	if ( version_compare( $wp_version, WP_SECRETS_API_CORE_VERSION, '>=' ) && $symbol_taken ) {
+	if ( version_compare( $release, WP_SECRETS_API_CORE_VERSION, '>=' ) && $symbol_taken ) {
 		add_action( 'admin_notices', 'wp_secrets_api_notice_superseded' );
 
 		return;

src/wp-admin/includes/secrets-site-health.php

@@ -85,7 +85,7 @@ function wp_secrets_site_health_result( string $test, string $label, string $sta
 function wp_secrets_site_health_test_key_source(): array {
 	$key_source = _wp_secrets_get_key_manager()->get_keyring()->get_key_source();
 
-	if ( false !== strpos( $key_source, 'broken' ) ) {
+	if ( str_contains( $key_source, 'broken' ) ) {
 		return wp_secrets_site_health_result(
 			'secrets_api_key_source',
 			__( 'Secrets API key source is broken' ),
@@ -97,7 +97,7 @@ function wp_secrets_site_health_test_key_source(): array {
 		);
 	}
 
-	if ( false !== strpos( $key_source, 'legacy interpretation' ) ) {
+	if ( str_contains( $key_source, 'legacy interpretation' ) ) {
 		return wp_secrets_site_health_result(
 			'secrets_api_key_source',
 			__( 'Secrets API key should be regenerated' ),
@@ -109,7 +109,7 @@ function wp_secrets_site_health_test_key_source(): array {
 		);
 	}
 
-	if ( false !== strpos( $key_source, 'LOGGED_IN_KEY' ) ) {
+	if ( str_contains( $key_source, 'LOGGED_IN_KEY' ) ) {
 		return wp_secrets_site_health_result(
 			'secrets_api_key_source',
 			__( 'Secrets API is using a fallback key' ),

src/wp-includes/secrets/class-wp-secrets-config-key-provider.php

@@ -261,13 +261,10 @@ private function get_site_key() {
 	 * @phpstan-assert-if-true non-empty-string $logged_in_salt
 	 */
 	private function are_usable_salt_values( $logged_in_key, $logged_in_salt ): bool {
-		foreach ( array( $logged_in_key, $logged_in_salt ) as $value ) {
-			if ( ! is_string( $value ) || '' === $value || self::KNOWN_PLACEHOLDER === $value ) {
-				return false;
-			}
-		}
-
-		return true;
+		return array_all(
+			array( $logged_in_key, $logged_in_salt ),
+			fn( $value ) => is_string( $value ) && '' !== $value && self::KNOWN_PLACEHOLDER !== $value
+		);
 	}
 
 	/**