Add a core/settings-update ability

[jorgefilipecosta]

What?

Adds core/settings-update next to core/settings-get. Based on #1087.

Why?

Agents can read the settings exposed to abilities but not change them. This lets them update those settings under the same rules as the REST API.

How?

• The ability lives in includes/Abilities/Settings/Settings.php, behind the Custom Abilities experiment with core/settings-get. Neither ability registers when no setting is exposed.
• Input: a map of setting name to its new value, for any setting core/settings-get reads except siteurl and admin_email, which stay read-only for now: a wrong siteurl makes wp-admin unreachable, and wp-admin only changes admin_email after the new address confirms it. null deletes the stored value, so the setting falls back to its default. Unknown names and empty input are rejected.
• It mirrors WP_REST_Settings_Controller::update_item(). Every value is sanitized against its schema before any is written; the REST API does this through each setting's sanitize_callback arg option, while the Abilities API only validates. Settings are written in registration order, objects reject undeclared properties, and null is refused with settings_invalid_stored_value while the stored value fails validation. Unlike the endpoint, which checks stored values while writing and keeps the settings it already wrote, every check runs before any write, so an error leaves every setting unchanged. Error codes use settings_ in place of rest_, and the REST-only filters do not run.
• Output: the map core/settings-get returns, as the endpoint answers with the whole settings object. core/settings-get now leaves out a value its schema rejects instead of failing for every setting. Without that, resetting a setting that has no default, such as default_ping_status, to null would break both abilities.
• It is destructive but not idempotent, so it stays on POST: the DELETE method cannot carry null.
• The tests port the settings endpoint's update tests under their core names. A throwaway harness ran 23 updates through both the endpoint and the ability on WordPress 7.1.2. The results match, apart from the differences above (unknown or empty input, admin_email, nothing saved when a null is refused) and values core/settings-get already reads differently (an unset array setting reads as [] instead of null).

Testing Instructions

1. Enable the Custom Abilities experiment.
2. Run core/settings-update with { "blogname": "New name" }, then with { "posts_per_page": null }. Each call returns the settings, with the new title and then posts_per_page back to 10. { "siteurl": "https://example.com" } is rejected.
3. npm run test:php -- --filter 'SettingsTest|Gated_AbilitiesTest|Custom_AbilitiesTest'
4. npm run test:e2e -- tests/e2e/specs/abilities/core-settings-update.spec.js

Changelog Entry

Added - New core/settings-update ability that mirrors the REST API settings endpoint.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.

---

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

[codecov]

Codecov Report

❌ Patch coverage is 98.92473% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.67%. Comparing base (2734238) to head (19dbe6f).

Files with missing lines	Patch %	Lines
includes/Abilities/Settings/Settings.php	98.92%	1 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                            @@
##             update/rename-settings-get-ability     #764      +/-   ##
========================================================================
+ Coverage                                 81.54%   81.67%   +0.13%     
- Complexity                                 3071     3092      +21     
========================================================================
  Files                                       129      129              
  Lines                                     12254    12341      +87     
========================================================================
+ Hits                                       9992    10080      +88     
+ Misses                                     2262     2261       -1     

Flag	Coverage Δ
unit	81.67% <98.92%> (+0.13%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jeffpaul]

I'd like to see #863 land alongside this specific PR (and any others that aren't explicitly read abilities), to help ensure folks enabling these sorts of abilities are doing so knowingly.

[gziolo]

Thanks for addressing my earlier feedback. I tested both abilities on my local site, and they behave the same as /wp/v2/settings for all the cases I tried: normal updates, null resetting to the default, invalid values, unknown settings, empty input, and permissions.

I pushed a commit with a few small fixes:

• Renamed the gated class from Settings_Get to Settings, since it now covers both abilities.
• The e2e tests now restore the settings in finally, so a failed assertion doesn't affect other tests.
• The core/settings-get description now says that a setting with an invalid stored value is left out of the result.

[gziolo]

When core/settings-update returns settings_invalid_stored_value, the settings registered before the failing one are already saved. Here's a small change (might need some enhancements) that runs every check before any write, so an error always means nothing was saved:

@@ execute_update_settings()
 		$options        = array();
 		$invalid_params = array();
+		$invalid_stored = '';
 		foreach ( (array) $this->exposed_settings as $name => $setting ) {
@@
-			// The endpoint's sanitize callback keeps null as is.
-			if ( ! is_null( $args['value'] ) ) {
+			if ( is_null( $args['value'] ) ) {
+				/*
+				 * As in the settings endpoint, a stored value that does not pass validation
+				 * cannot be updated to null. The endpoint returns such values as null, so this
+				 * keeps a client that sends a response back from deleting them by mistake.
+				 * The endpoint checks this while writing; checking it here keeps the earlier
+				 * settings in the input from being written when the update fails.
+				 */
+				if ( '' === $invalid_stored && is_wp_error( rest_validate_value_from_schema( get_option( $args['option_name'], false ), $args['schema'] ) ) ) {
+					$invalid_stored = $name;
+				}
+			} else {
+				// The endpoint's sanitize callback keeps null as is, and sanitizes anything else.
 				$args['value'] = rest_sanitize_value_from_schema( $args['value'], $args['schema'], $name );
 			}
@@
-		foreach ( $options as $name => $args ) {
+		if ( '' !== $invalid_stored ) {
+			return new WP_Error(
+				'settings_invalid_stored_value',
+				/* translators: %s: Property name. */
+				sprintf( __( 'The %s property has an invalid stored value, and cannot be updated to null.', 'ai' ), $invalid_stored ),
+				array( 'status' => 500 )
+			);
+		}
+
+		foreach ( $options as $args ) {
 			/*
 			 * A null value for an option would have the same effect as
@@
 			if ( is_null( $args['value'] ) ) {
-				/*
-				 * As in the settings endpoint, a stored value that does not pass validation
-				 * cannot be updated to null. The endpoint returns such values as null, so this
-				 * keeps a client that sends a response back from deleting them by mistake.
-				 */
-				if ( is_wp_error( rest_validate_value_from_schema( get_option( $args['option_name'], false ), $args['schema'] ) ) ) {
-					return new WP_Error(
-						'settings_invalid_stored_value',
-						/* translators: %s: Property name. */
-						sprintf( __( 'The %s property has an invalid stored value, and cannot be updated to null.', 'ai' ), $name ),
-						array( 'status' => 500 )
-					);
-				}
-
 				delete_option( $args['option_name'] );

Invalid parameters (400) are still reported first, same as now. The stored value error (500) comes next, and only then do we write.

This changes test_core_settings_update_writes_in_registration_order, which asserts the partial save on purpose. It would become test_core_settings_update_refused_null_writes_no_setting, with the last assertion flipped to 'Original Name'. The downside is that no test covers the write order anymore. I think that's fine, because the order is hard to observe once partial saves are gone.

This is an intentional difference from /wp/v2/settings, so it would be good to mention it in the PR description.

Happy to push it if you agree.

[gziolo]

Should we opt out sensitive settings from core/settings-update? I mean these two:

• siteurl: a wrong value can make the site unreachable.
• admin_email: wp-admin asks for email confirmation before changing it, but the ability skips that step.

They could stay readable with core/get-site-info. The REST API isn't this strict and allows both, but agents make mistakes more easily, so I'd rather be careful here.

[jorgefilipecosta]

Should we opt out sensitive settings from core/settings-update? I mean these two:

• siteurl: a wrong value can make the site unreachable.
• admin_email: wp-admin asks for email confirmation before changing it, but the ability skips that step.

They could stay readable with core/get-site-info. The REST API isn't this strict and allows both, but agents make mistakes more easily, so I'd rather be careful here.

Digging deeper into I applied the suggest admin_email for example on normal UI requires an email confirmation, If we support agent changes I guess we should also include that confirmation, changing site url is also risky. I left both settings out for now we can change that later.

[jorgefilipecosta]

When core/settings-update returns settings_invalid_stored_value, the settings registered before the failing one are already saved. Here's a small change (might need some enhancements) that runs every check before any write, so an error always means nothing was saved:

@@ execute_update_settings()
 		$options        = array();
 		$invalid_params = array();
+		$invalid_stored = '';
 		foreach ( (array) $this->exposed_settings as $name => $setting ) {
@@
-			// The endpoint's sanitize callback keeps null as is.
-			if ( ! is_null( $args['value'] ) ) {
+			if ( is_null( $args['value'] ) ) {
+				/*
+				 * As in the settings endpoint, a stored value that does not pass validation
+				 * cannot be updated to null. The endpoint returns such values as null, so this
+				 * keeps a client that sends a response back from deleting them by mistake.
+				 * The endpoint checks this while writing; checking it here keeps the earlier
+				 * settings in the input from being written when the update fails.
+				 */
+				if ( '' === $invalid_stored && is_wp_error( rest_validate_value_from_schema( get_option( $args['option_name'], false ), $args['schema'] ) ) ) {
+					$invalid_stored = $name;
+				}
+			} else {
+				// The endpoint's sanitize callback keeps null as is, and sanitizes anything else.
 				$args['value'] = rest_sanitize_value_from_schema( $args['value'], $args['schema'], $name );
 			}
@@
-		foreach ( $options as $name => $args ) {
+		if ( '' !== $invalid_stored ) {
+			return new WP_Error(
+				'settings_invalid_stored_value',
+				/* translators: %s: Property name. */
+				sprintf( __( 'The %s property has an invalid stored value, and cannot be updated to null.', 'ai' ), $invalid_stored ),
+				array( 'status' => 500 )
+			);
+		}
+
+		foreach ( $options as $args ) {
 			/*
 			 * A null value for an option would have the same effect as
@@
 			if ( is_null( $args['value'] ) ) {
-				/*
-				 * As in the settings endpoint, a stored value that does not pass validation
-				 * cannot be updated to null. The endpoint returns such values as null, so this
-				 * keeps a client that sends a response back from deleting them by mistake.
-				 */
-				if ( is_wp_error( rest_validate_value_from_schema( get_option( $args['option_name'], false ), $args['schema'] ) ) ) {
-					return new WP_Error(
-						'settings_invalid_stored_value',
-						/* translators: %s: Property name. */
-						sprintf( __( 'The %s property has an invalid stored value, and cannot be updated to null.', 'ai' ), $name ),
-						array( 'status' => 500 )
-					);
-				}
-
 				delete_option( $args['option_name'] );

Invalid parameters (400) are still reported first, same as now. The stored value error (500) comes next, and only then do we write.

This changes test_core_settings_update_writes_in_registration_order, which asserts the partial save on purpose. It would become test_core_settings_update_refused_null_writes_no_setting, with the last assertion flipped to 'Original Name'. The downside is that no test covers the write order anymore. I think that's fine, because the order is hard to observe once partial saves are gone.

This is an intentional difference from /wp/v2/settings, so it would be good to mention it in the PR description.

Happy to push it if you agree.

Hi @gziolo feel free to push any changes.

[jorgefilipecosta]

Hi @gziolo I applied all the feedback.