Repository navigation
Conversation
Without `WP_SECRETS_KEY`, the default keyring derived its key from `LOGGED_IN_KEY` and `LOGGED_IN_SALT`. Rotating the salts, which is routine advice, then made every stored secret unreadable. The constants were also read directly rather than through `wp_salt()`, so a translated placeholder from a localized `wp-config-sample.php`, or a value shared by several of the salts, became the source of the key. There is no fallback any more. Without a usable `WP_SECRETS_KEY`, every call that needs the key returns `secret_key_unavailable`. A key equal to the `wp-config-sample.php` placeholder is not usable, and neither is one equal to any of the authentication keys and salts. An unfilled sample gives all of them the same phrase, so the second check rejects the placeholder in any language. The Site Health key source test now reports a missing or unusable key as recommended, with a newly generated line to add to `wp-config.php`. Follow-up to [64337]. See #66187, #66399.
…ealth. 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 <noreply@anthropic.com>
…s 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…tored secrets. Addresses review. Only the default keyring reads `wp-config.php`, so the critical key source result names `WP_SECRETS_KEY` only for it. A keyring from a `secrets.php` drop-in, such as a key service that is unreachable or was installed without re-wrapping the root key, has its own error shown instead. The label no longer says the key "does not match", which does not fit an outage. With `WP_SECRETS_KEY` now required, the key source and undecryptable results no longer mention `LOGGED_IN_KEY` and `LOGGED_IN_SALT`. When a drop-in replaces the provider, Site Health Info marks the keyring class and key source as not used, instead of describing a keyring that the site does not use. See #66380, #66187.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Resolves the conflicts with r65272: the tests this branch changes now follow the same conventions, with a summary, `@covers` and a failure message for each assertion.
`WP_Secrets_Config_Key_Provider::get_key_source()` returned English text that Site Health showed as is and also searched for "not defined", "not usable" and "legacy interpretation" to choose its result. The text could not be translated without breaking those checks. The provider now reports the state of `WP_SECRETS_KEY` through `get_key_status()`, which returns one of four `KEY_STATUS_*` constants. Site Health uses that, and `get_key_source()` returns translated text.
Now that `WP_SECRETS_KEY` is required, a plugin needs to know whether the Secrets API can be used before moving a credential into it. This returns false when the secrets.php drop-in failed to load, or when the default keyring has no usable key. A provider or keyring from a working drop-in manages its own keys, so it is reported as available. Suggested by peterwilsoncc in review.
Brings in trunk through r65272 and the updated WordPress#15258. The key source test now uses `WP_Secrets_Config_Key_Provider::get_key_status()` rather than the text of the key source, and the Site Health tests this branch adds or changes follow r65272's conventions: a summary, `@covers`, and a failure message for each assertion.
Author
…te Health. A WP_SECRETS_KEY of only whitespace was accepted as key material. It is now treated like an empty key. Site Health wraps WP_SECRETS_KEY, wp-config.php and wp-config-sample.php in <code>, as the other Site Health results do, and quotes the translated "stop editing" line the way network.php does. From review on PR 15258 by peterwilsoncc.
… the root key. A site that stored secrets and then added or changed WP_SECRETS_KEY without re-wrapping the root key has a usable-looking key, but every read and write fails. wp_is_secrets_api_available() now checks the stored root key, so a caller that checks first keeps its existing storage instead of failing on the write. With no root key stored yet, the API is still available: the first write creates one under the key. See #66187 comment:56.
…lts. Wraps WP_SECRETS_KEY and secrets.php in <code>, as the other Site Health results do and as PR 15258 now does for the strings it adds. The provider label and the keyring's error message are escaped on their own. From review on PR 15258 by peterwilsoncc.
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.
Takes over #14847 by @mpeshev, as announced on the ticket. Parts 1 to 3 of #66380 are fixed in mpeshev's three commits, which are unchanged apart from a rebase. One commit of mine on top makes the change requested in review.
This is based on #15258 (require
WP_SECRETS_KEY, #66187) and includes its commit. Please review the last four commits. Once #15258 lands, I'll update this so it contains only these.What changes
From #14847 (mpeshev):
WP_Errorfromwp_list_secrets(). They no longer treat it as an empty list.secrets.phpreplaces the provider, the key source test says the provider manages its keys and doesn't mentionWP_SECRETS_KEY.wp secret generate-key, which isn't in core or WP-CLI.My commit, from review:
WP_SECRETS_KEYonly for the default keyring. For a keyring from a drop-in, it shows that keyring's own error. Examples are a KMS that's unreachable, or one installed without re-wrapping the root key. The label is now "Secrets API cannot unlock the stored secrets", because "does not match" doesn't fit an outage.LOGGED_IN_KEYorLOGGED_IN_SALTin the text. With Secrets API: RequireWP_SECRETS_KEY#15258 they no longer have anything to do with the key. That also covers @Clorith's request on the ticket for simpler text: a missing key comes with the line to add.Part 4, listing
secrets.phpas a drop-in, is still #14848. It's unaffected and ready.Testing
--group secrets: 432 tests on single site and 440 on multisite, no failures (PHP 8.5, trunk at r65378, with Secrets API: RequireWP_SECRETS_KEY#15258).Mock_Keyring::configure_fail_unwrap()WP_SECRETS_KEY#15258 also still pass.Trac ticket: https://core.trac.wordpress.org/ticket/66380
Props mpeshev, nofearinc, ravipatel, dhrupo, Clorith.
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: The original commits were written with Claude Code, as noted on #14847. For this takeover: the rebase, the change from review, the tests and this description. I reviewed the changes and the results.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.