Add Settings API #331
Open
Add Settings API #331
Conversation
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
Contributor
📝 WalkthroughWalkthroughAdds a new Settings REST API endpoint with CRUD operations and sensitive-key protections, relocates the error-reporting setting to General, improves Vue compatibility for form fields, enhances addon error logging, and exposes a filter for allowed HTML tags. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as REST Client
participant Endpoint as Settings_Endpoint
participant API as API (Auth)
participant Settings as Settings Store
participant Filters as Filters/Hooks
Client->>Endpoint: Request (GET/POST/PUT /settings...)
Endpoint->>API: authorize_request()
API-->>Endpoint: auth result
alt authorized
Endpoint->>Filters: apply filters (e.g., wu_api_sensitive_settings)
Filters-->>Endpoint: filtered rules
Endpoint->>Settings: read/update settings
Settings-->>Endpoint: result (data / success / failures)
Endpoint->>Filters: maybe_log_api_call()
Endpoint-->>Client: HTTP response (200 / 400 / 403 / 404 / 500)
else unauthorized
Endpoint-->>Client: 403 Forbidden
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/functions/helper.php (1)
498-625: Fix array union operator with numeric keys to include all intended tags.The array union operator (
+) preserves the left-hand array's values for overlapping numeric keys. Sincearray_keys($allowed_html)returns numeric keys[0, 1, ...]and the extra tags array also has numeric keys[0, 1, 2, ...], the+operator will drop extra tags that occupy indices already present inarray_keys()(specifically 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input'). Usearray_merge()instead to concatenate the arrays and ensure all tags are included.🔧 Suggested fix
- ] + array_merge_recursive($allowed_html, array_fill_keys(array_keys($allowed_html) + ['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input', 'textarea'], $vue_and_data_attributes)); + ] + array_merge_recursive( + $allowed_html, + array_fill_keys( + array_unique(array_merge(array_keys($allowed_html), ['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input', 'textarea'])), + $vue_and_data_attributes + ) + );
🤖 Fix all issues with AI agents
In `@inc/apis/class-settings-endpoint.php`:
- Around line 258-270: The loop that calls wu_save_setting() with raw $value
should sanitize/validate via the Field API first: implement a helper (e.g.
sanitize_setting_value(string $key, $value)) that iterates
Settings::get_instance()->get_sections(), finds the field definition for $key,
constructs a \WP_Ultimo\UI\Field($key, $field_def), calls
$field->set_value($value) and returns $field->get_value(); then replace the
direct calls to wu_save_setting($key, $value) in the foreach over
$filtered_settings (and the similar persistence block later) to call $sanitized
= sanitize_setting_value($key, $value) and either pass $sanitized to
wu_save_setting or skip/reject unknown keys (returning null) as appropriate.
- Around line 426-436: The maybe_log_api_call method currently logs raw request
bodies via $request->get_body() which can leak sensitive data; before building
the $payload (or before calling wp_json_encode) sanitize the body by removing or
replacing sensitive keys (e.g., "password", "token", "authorization", "api_key",
"credit_card", "ssn") or omit the body entirely, e.g., call a helper like
redact_request_body($request->get_body()) and use that redacted result for the
'body_params' value so wp_json_encode(wu_log_add(...)) never receives plaintext
secrets; update maybe_log_api_call to perform this redaction and ensure
get_body()/body_params references the sanitized output.
In `@inc/class-settings.php`:
- Around line 711-720: The privacy-policy link in the description passed to
add_field for 'enable_error_reporting' lacks rel="noopener noreferrer" and the
URL isn't escaped; update the sprintf call building the description so the <a
href="%s" ...> uses esc_url( 'https://ultimatemultisite.com/privacy-policy/' )
and add rel="noopener noreferrer" to the anchor (i.e., modify the description
string for add_field('general','enable_error_reporting', ...) to include
rel="noopener noreferrer" and wrap the URL with esc_url()).
In `@readme.txt`:
- Around line 243-249: The changelog contains duplicate "Version [2.4.10]"
entries (the headers "Version [2.4.10]" and the block dated "2026-XX-XX"); merge
the two blocks into a single 2.4.10 entry combining both bullet points, and
replace the placeholder date "2026-XX-XX" with a concrete release date (e.g.,
"2026-01-25") or label the entry "Unreleased" if it isn't finalized; update the
single "Version [2.4.10]" header and remove the redundant header/block so only
one consolidated 2.4.10 section remains.
🧹 Nitpick comments (1)
inc/apis/class-settings-endpoint.php (1)
409-412: Drop unused$valueto satisfy PHPMDLine 409 declares
$valuebut never uses it. Usingforeach (array_keys(...))or$key => $_avoids the PHPMD warning.🧹 Suggested tidy‑up
- foreach ($settings as $key => $value) { + foreach ($settings as $key => $_) { if ($this->is_sensitive_setting($key)) { unset($settings[ $key ]); } }
Comment on lines
+258
to
+270
| // Save each setting | ||
| $updated = []; | ||
| $failed = []; | ||
|
|
||
| foreach ($filtered_settings as $key => $value) { | ||
| $result = wu_save_setting($key, $value); | ||
|
|
||
| if ($result) { | ||
| $updated[] = $key; | ||
| } else { | ||
| $failed[] = $key; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Validate/sanitize values before wu_save_setting
Lines 263 and 327 persist raw values, bypassing the Field-based sanitization used by the UI flow. This can store invalid or unsafe values (especially for text/HTML fields). Consider resolving the field definition and running Field::set_value() before saving, or rejecting unknown keys.
🛠️ Proposed fix (sanitize via Field)
- $result = wu_save_setting($key, $value);
+ $sanitized_value = $this->sanitize_setting_value($key, $value);
+ $result = wu_save_setting($key, $sanitized_value);- $result = wu_save_setting($setting_key, $value);
+ $sanitized_value = $this->sanitize_setting_value($setting_key, $value);
+ $result = wu_save_setting($setting_key, $sanitized_value);You’d need a helper such as:
protected function sanitize_setting_value(string $key, $value) {
$sections = \WP_Ultimo\Settings::get_instance()->get_sections();
foreach ($sections as $section) {
if (isset($section['fields'][$key])) {
$field = new \WP_Ultimo\UI\Field($key, $section['fields'][$key]);
$field->set_value($value);
return $field->get_value();
}
}
return $value; // or return null to reject unknown keys
}Also applies to: 327-338
🤖 Prompt for AI Agents
In `@inc/apis/class-settings-endpoint.php` around lines 258 - 270, The loop that
calls wu_save_setting() with raw $value should sanitize/validate via the Field
API first: implement a helper (e.g. sanitize_setting_value(string $key, $value))
that iterates Settings::get_instance()->get_sections(), finds the field
definition for $key, constructs a \WP_Ultimo\UI\Field($key, $field_def), calls
$field->set_value($value) and returns $field->get_value(); then replace the
direct calls to wu_save_setting($key, $value) in the foreach over
$filtered_settings (and the similar persistence block later) to call $sanitized
= sanitize_setting_value($key, $value) and either pass $sanitized to
wu_save_setting or skip/reject unknown keys (returning null) as appropriate.
Comment on lines
+426
to
+436
| protected function maybe_log_api_call($request): void { | ||
|
|
||
| if (\WP_Ultimo\API::get_instance()->should_log_api_calls()) { | ||
| $payload = [ | ||
| 'route' => $request->get_route(), | ||
| 'method' => $request->get_method(), | ||
| 'url_params' => $request->get_url_params(), | ||
| 'body_params' => $request->get_body(), | ||
| ]; | ||
|
|
||
| wu_log_add('api-calls', wp_json_encode($payload, JSON_PRETTY_PRINT)); |
Contributor
There was a problem hiding this comment.
Redact request bodies before logging
Line 433–434 logs the raw request body. Even if sensitive keys are rejected later, their values can still be written to logs. Please redact sensitive keys (or omit body logging entirely) before logging.
🛠️ Proposed fix (redact body params)
protected function maybe_log_api_call($request): void {
if (\WP_Ultimo\API::get_instance()->should_log_api_calls()) {
+ $body_params = $request->get_json_params();
+ if (empty($body_params)) {
+ $body_params = $request->get_body_params();
+ }
+ if (isset($body_params['settings']) && is_array($body_params['settings'])) {
+ $body_params['settings'] = $this->filter_sensitive_settings($body_params['settings']);
+ }
+ $setting_key = $request->get_param('setting_key');
+ if ($setting_key && $this->is_sensitive_setting($setting_key)) {
+ $body_params = ['redacted' => true];
+ }
$payload = [
'route' => $request->get_route(),
'method' => $request->get_method(),
'url_params' => $request->get_url_params(),
- 'body_params' => $request->get_body(),
+ 'body_params' => $body_params,
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| protected function maybe_log_api_call($request): void { | |
| if (\WP_Ultimo\API::get_instance()->should_log_api_calls()) { | |
| $payload = [ | |
| 'route' => $request->get_route(), | |
| 'method' => $request->get_method(), | |
| 'url_params' => $request->get_url_params(), | |
| 'body_params' => $request->get_body(), | |
| ]; | |
| wu_log_add('api-calls', wp_json_encode($payload, JSON_PRETTY_PRINT)); | |
| protected function maybe_log_api_call($request): void { | |
| if (\WP_Ultimo\API::get_instance()->should_log_api_calls()) { | |
| $body_params = $request->get_json_params(); | |
| if (empty($body_params)) { | |
| $body_params = $request->get_body_params(); | |
| } | |
| if (isset($body_params['settings']) && is_array($body_params['settings'])) { | |
| $body_params['settings'] = $this->filter_sensitive_settings($body_params['settings']); | |
| } | |
| $setting_key = $request->get_param('setting_key'); | |
| if ($setting_key && $this->is_sensitive_setting($setting_key)) { | |
| $body_params = ['redacted' => true]; | |
| } | |
| $payload = [ | |
| 'route' => $request->get_route(), | |
| 'method' => $request->get_method(), | |
| 'url_params' => $request->get_url_params(), | |
| 'body_params' => $body_params, | |
| ]; | |
| wu_log_add('api-calls', wp_json_encode($payload, JSON_PRETTY_PRINT)); |
🤖 Prompt for AI Agents
In `@inc/apis/class-settings-endpoint.php` around lines 426 - 436, The
maybe_log_api_call method currently logs raw request bodies via
$request->get_body() which can leak sensitive data; before building the $payload
(or before calling wp_json_encode) sanitize the body by removing or replacing
sensitive keys (e.g., "password", "token", "authorization", "api_key",
"credit_card", "ssn") or omit the body entirely, e.g., call a helper like
redact_request_body($request->get_body()) and use that redacted result for the
'body_params' value so wp_json_encode(wu_log_add(...)) never receives plaintext
secrets; update maybe_log_api_call to perform this redaction and ensure
get_body()/body_params references the sanitized output.
Comment on lines
+711
to
+720
| $this->add_field( | ||
| 'general', | ||
| 'enable_error_reporting', | ||
| [ | ||
| 'title' => __('Help Improve Ultimate Multisite', 'ultimate-multisite'), | ||
| 'desc' => sprintf( | ||
| /* translators: %s is a link to the privacy policy */ | ||
| __('Allow Ultimate Multisite to collect anonymous usage data and error reports to help us improve the plugin. We collect: PHP version, WordPress version, plugin version, network type (subdomain/subdirectory), aggregate counts (sites, memberships), active gateways, and error logs. We never collect personal data, customer information, or domain names. <a href="%s" target="_blank">Learn more</a>.', 'ultimate-multisite'), | ||
| 'https://ultimatemultisite.com/privacy-policy/' | ||
| ), |
Contributor
There was a problem hiding this comment.
Add rel="noopener noreferrer" to the privacy‑policy link
Line 718 opens a new tab without rel, which exposes a tabnabbing risk. Consider also wrapping the URL with esc_url() for safety.
🛠️ Proposed fix
- 'desc' => sprintf(
+ 'desc' => sprintf(
/* translators: %s is a link to the privacy policy */
- __('Allow Ultimate Multisite to collect anonymous usage data and error reports to help us improve the plugin. We collect: PHP version, WordPress version, plugin version, network type (subdomain/subdirectory), aggregate counts (sites, memberships), active gateways, and error logs. We never collect personal data, customer information, or domain names. <a href="%s" target="_blank">Learn more</a>.', 'ultimate-multisite'),
- 'https://ultimatemultisite.com/privacy-policy/'
+ __('Allow Ultimate Multisite to collect anonymous usage data and error reports to help us improve the plugin. We collect: PHP version, WordPress version, plugin version, network type (subdomain/subdirectory), aggregate counts (sites, memberships), active gateways, and error logs. We never collect personal data, customer information, or domain names. <a href="%s" target="_blank" rel="noopener noreferrer">Learn more</a>.', 'ultimate-multisite'),
+ esc_url('https://ultimatemultisite.com/privacy-policy/')
),🤖 Prompt for AI Agents
In `@inc/class-settings.php` around lines 711 - 720, The privacy-policy link in
the description passed to add_field for 'enable_error_reporting' lacks
rel="noopener noreferrer" and the URL isn't escaped; update the sprintf call
building the description so the <a href="%s" ...> uses esc_url(
'https://ultimatemultisite.com/privacy-policy/' ) and add rel="noopener
noreferrer" to the anchor (i.e., modify the description string for
add_field('general','enable_error_reporting', ...) to include rel="noopener
noreferrer" and wrap the URL with esc_url()).
Comment on lines
+243
to
249
| Version [2.4.10] - Released on 2026-XX-XX | ||
| - New: Settings API | ||
| - Fix: Problems with choosing country and state | ||
|
|
||
|
|
||
| Version [2.4.10] - Released on 2026-01-23 | ||
| - New: Configurable minimum password strength setting with Medium, Strong, and Super Strong options. |
Contributor
There was a problem hiding this comment.
Changelog duplicates 2.4.10 and uses a placeholder date
Lines 243–248 introduce a second 2.4.10 header with “2026‑XX‑XX”. Please merge into a single 2.4.10 entry and use a concrete release date (e.g., 2026‑01‑25 if releasing today) or mark as “Unreleased”.
🤖 Prompt for AI Agents
In `@readme.txt` around lines 243 - 249, The changelog contains duplicate "Version
[2.4.10]" entries (the headers "Version [2.4.10]" and the block dated
"2026-XX-XX"); merge the two blocks into a single 2.4.10 entry combining both
bullet points, and replace the placeholder date "2026-XX-XX" with a concrete
release date (e.g., "2026-01-25") or label the entry "Unreleased" if it isn't
finalized; update the single "Version [2.4.10]" header and remove the redundant
header/block so only one consolidated 2.4.10 section remains.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
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.
and a few other tweaks
closes #330
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.