Repository navigation
Conversation
Ports r64344 and r64345 from core trunk. The Site Health key source checks, WP_Secrets_Libsodium_Provider::list_secrets(), Mock_Keyring and Mock_Store compare with str_contains() and str_starts_with() instead of strpos(). Core polyfills both since 5.9, below the plugin's 6.6 minimum. PHPStan analyzes at PHP 7.4 and the WordPress stubs leave out compat.php, so tests/phpstan/compat-stubs.php declares the two polyfills with compat.php's signatures. Props soean. See Trac #66187.
Ports r64674 from core trunk. Saving a secret's current value again demoted it to the previous slot, and settings forms do that on every submit, so a rotation in progress lost the previous value it depends on. WP_Secrets_Libsodium_Provider::set() now treats an unchanged write as a no-op: nothing is stored and wp_secret_changed does not fire. A write that raises needs_rotation on an unflagged value, such as wp_import_option_as_secret() on an option holding the current value, is still recorded, and keeps the previous version. WP_Secrets_Provider::set() documents the rule for every provider, and WP_Secrets_Provider_Conformance tests it. The versioning, rotation and extension-point pages describe it. PHPStan at level 10 loses the narrowing of $existing behind $is_unchanged; the one line is ignored in phpstan.neon.dist so src/ stays as core committed it. Props ravipatel, ericmann, whyisjake. See Trac #66353, #66187.
The conformance test added with r64674 fails both example providers. Vault's KV v2 engine adds version N+1 for any write, and PutSecretValue moves AWSPREVIOUS onto the value AWSCURRENT already holds, so re-saving the current value pushed the real previous one out. Both now read the current value first and skip a write that would not change it. Vault_KV2_Provider still records a raised rotation flag on an unchanged value, as a metadata write with no new version, and reports it. A failed read does not count as unchanged: the write goes ahead and reports its own error.
Ports the Secrets API part of r64675 from core trunk. Test-only, with no change in behavior: a stored record is always an array, never null. Props Soean, huzaifaalmesbah.
Ports r64690 from core trunk: are_usable_salt_values() checks both salt values with array_all() instead of a foreach. Core polyfills array_all() from 6.8 (r59783), so 6.6 and 6.7 would have fataled on the default keyring. The plugin now requires 6.8, rather than carry a polyfill of its own. The floor was never tested: CI ran only latest and trunk. A PHP 7.4 / WP 6.8 leg now covers it. tests/phpstan/compat-stubs.php declares array_all() with compat.php's signature. Props soean.
version_compare() ranks '7.2-beta1-64606-src' below '7.2', so on a 7.2 beta, RC, or trunk build that ships the API, the bootstrap took the conflict branch. It still loaded nothing, but showed the error notice saying another plugin had declared wp_get_secret(). The version test now compares the release part only.
Core's test bootstrap has loaded its own copies of the Secrets API test helpers since r64337, so every trunk job fataled redeclaring WP_Secrets_Assertions. The plugin's copies now load only when the WordPress test library has none. With that fixed, the core-bound tests pass on trunk against core's own implementation. The plugin-only layer (the WP-CLI commands, the migrator, the legacy reader, and the prototype fallback store) does not load once the plugin stands down, so those five test classes skip on a WordPress whose wp-includes/secrets.php declares wp_get_secret(). The skip keys on that, not on a missing class, so a plugin that failed to load its layer on 6.8 still fails.
ericmann
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings core trunk's Secrets API changes since r64337 into the plugin.
src/and the shared test helpers now match trunk byte for byte. It also gets the trunk CI legs passing: they had been fataling since the API landed in core.Core changesets ported
str_contains()/str_starts_with()in Site Health andlist_secrets()Mock_KeyringandMock_Store??inMock_Store::get()(test-only; found in the parity check)array_all()in the config keyring (found in the parity check)WordPress 6.8 is now the minimum
r64690 uses
array_all(), which core polyfills only from 6.8 (r59783). On 6.6 or 6.7 the default keyring would fatal. Rather than carry a polyfill of our own, the plugin now requires 6.8: the header, the README, and the scope page are updated. The old floor was never tested, since CI ran onlylatestandtrunk. A PHP 7.4 / WP 6.8 leg now covers the new one.Trunk: the plugin stands down, and the suite runs
Cannot declare trait WP_Secrets_Assertions. The plugin's copies now load only when the test library has none. With that fixed, every core-bound test passes on trunk against core's own implementation.wp-includes/secrets.phpdeclaringwp_get_secret(), not on a missing plugin class. A plugin that failed to load its layer on 6.8 still fails.version_compare()ranks7.2-beta1-64606-srcbelow7.2. On a 7.2 beta, RC, or trunk build, the bootstrap therefore showed the error notice ("another plugin has already declaredwp_get_secret()") instead of the superseded one. It now compares the release part only. Two new no-op gate tests cover pre-releases.Follow-up, not in this PR: on 7.2 the plugin should keep offering the WP-CLI commands, running on core's classes instead of its own. Today it goes fully dark, which is enough to make this PR clean.
Beyond the straight ports
PutSecretValuemoves AWSPREVIOUS onto the unchanged value. Both now read the current value and skip an unchanged write. Vault still records a newly raised rotation flag, as a metadata-only write that fires the hook. A failed read never counts as "unchanged". There are three new Vault unit tests. The conformance test fails on the old examples and passes on the new ones.compat.php.tests/phpstan/compat-stubs.phpdeclares the three polyfillssrc/now uses, withcompat.php's signatures. r64674 also trips one level-10offsetAccess.nonOffsetAccessible, because PHPStan doesn't narrow$existingthrough$is_unchanged. That one line is ignored inphpstan.neon.distrather than rewritten, to keepsrc/identical to core. A core follow-up could guard the check with$has_current &&.examples/README.md, are updated.docs/reference/classes.mdis regenerated.Testing
main, so the new skip hides nothing there. On trunk the extra skips are the five plugin-only classes.make lint compat analyse reference-checkall pass.main, because.wp-env.override.jsondefines live AWS constants.Props for the changelog
Ready for the next release's changelog page:
All committed by whyisjake.