Repository navigation
Conversation
Without `WP_SECRETS_KEY`, the Secrets API derives its key from `LOGGED_IN_KEY` and `LOGGED_IN_SALT`, so rotating the salts makes every stored secret unreadable. `setup-config.php` now writes a base64-encoded random 32-byte `WP_SECRETS_KEY` after the keys and salts, outside their group, whose advice to change them at any time does not apply to it. The key is left out when no CSPRNG is available, or when `wp-config-sample.php` already defines it. `wp-config-sample.php` itself is unchanged: the Secrets API accepts any `WP_SECRETS_KEY` string, so a placeholder there would become a public key on hand-edited installs. Follow-up to [64337]. See #66399, #66187, #66380.
|
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. |
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
This is working for a manual install but a fresh install via WP-CLI. It looks like an associated PR will need to be made to https://github.com/wp-cli/config-command, https://github.com/wp-cli/core-command or both.
Added some notes inline.
| * left out, and the Secrets API derives its key from those salts instead. | ||
| */ | ||
| try { | ||
| $secrets_key = base64_encode( random_bytes( 32 ) ); |
There was a problem hiding this comment.
The other values fall back to wp_generate_password() if CSPRING isn't available. It might be worth doing so here too.
On the ticket I've suggested coordinating with meta to have WP_SECRETS_KEY added to the secrets API to allow this to fall back to the API if needs be.
There was a problem hiding this comment.
Done in 6c93419. Without a CSPRNG it now uses wp_generate_password( 64, true, true ), like the salts. With the fallback removed in #15258, a weaker key is better than none. I've replied on Trac about the API: it should be a separate endpoint, because people paste the /1.1/salt/ output to rotate their salts.
| break; | ||
| } | ||
|
|
||
| if ( 'NONCE_SALT' === $constant ) { |
There was a problem hiding this comment.
This is a little flakey if additional secrets are added to the sample config file.
I also think it a good idea to add this to the sample config file but let me know if there are reasons you don't wish to do this.
I suggest adding this above the /**#@-*/ but separated from the group by some line breaks, so the output becomes:
/**#@+
* Authentication unique keys and salts.
*
* Change these to different unique phrases! You can generate these using
* the {@link https://api.wordpress.org/secret-key/1.1/salt/ WordPress.org secret-key service}.
*
* You can change these at any point in time to invalidate all existing cookies.
* This will force all users to have to log in again.
*
* @since 2.6.0
*/
define( 'AUTH_KEY', '...' );
define( 'SECURE_AUTH_KEY', '...' );
define( 'LOGGED_IN_KEY', '...' );
define( 'NONCE_KEY', '...' );
define( 'AUTH_SALT', '...' );
define( 'SECURE_AUTH_SALT', '...' );
define( 'LOGGED_IN_SALT', '...' );
define( 'NONCE_SALT', '...' );
/**
* Encryption key for the Secrets API.
*
* Secrets stored by WordPress and plugins are encrypted with this key.
* Unlike the keys and salts above, changing it makes every stored secret unreadable.
*
* @since 7.2.0
*/
define( 'WP_SECRETS_KEY', '...' );
/**#@-*/There was a problem hiding this comment.
Done. The sample now defines WP_SECRETS_KEY with the placeholder, and the loop that fills in the salts replaces it, so constants added to the sample later don't matter. A sample without the line still gets it added after the salts.
I put it after /**#@-*/ rather than inside the group. phpDocumentor applies the /**#@+ docblock to everything up to /**#@-*/, and that docblock says these values can be changed at any time, which is exactly what this key must not be. I'm happy to move it inside if you'd rather keep the keys together.
…sample.php`. Addresses review. `wp-config-sample.php` now defines `WP_SECRETS_KEY` with the placeholder, after the keys and salts and outside their group, and `setup-config.php` replaces it in the same way it fills in the salts. A custom or older localized sample without the line still gets it added after the salts. Without a CSPRNG the key comes from `wp_generate_password()`, as the salts do, rather than being left out. This depends on the Secrets API rejecting the placeholder, which the patch on #66187 that requires `WP_SECRETS_KEY` adds. See #66399, #66187.
Agreed. |
setup-config.phpnow writes a dedicatedWP_SECRETS_KEY, so new installs don't depend on the salts. Rotating the salts made every stored secret unreadable on a site that derived its key from them (#66187 comment:23, confirmed in comment:24). #15258 removes that fallback. Without a key, the Secrets API is then unavailable.This depends on #15258. It adds a placeholder to
wp-config-sample.php, and only #15258 makes the Secrets API reject it. Without that patch, the placeholder would become a public key.What changes
wp-config-sample.phpdefinesWP_SECRETS_KEYwith the placeholder after the keys and salts. It sits outside their/**#@+group, because the group's docblock says they can be changed at any time, and this key must not be.setup-config.phpfills it in. It replaces the placeholder line in the same loop that fills in the salts. The value is a base64-encoded random 32-byte key, the form the Secrets API uses as raw key material.wp_generate_password(), as the salts do.Existing sites are not affected. The Site Health side is #66380.
Testing
setup-config.phphas no unit tests. I tested it end to end on PHP 8.5 and MySQL 8:setup-config.php?step=2andinstall.php?step=2against a fresh database.wp config shuffle-saltsdoes.WP_SECRETS_KEYdefines inwp-config.phpsecret_key_unavailableAlso checked:
php -l.phpcsreports nothing new on the changed files.phpstan analysewith core's configuration passes.To test by hand:
wp-config.phpand run the installer.WP_SECRETS_KEYfollows the salts.wp config shuffle-salts.WP-CLI's
wp config createwrites its own file, so it needs the same change separately.Trac ticket: https://core.trac.wordpress.org/ticket/66399
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Drafting the patch, running the install tests above, and this description. I reviewed the change 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.