Security: require capability checks on privileged network-admin AJAX endpoints#1371
Merged
superdav42 merged 5 commits intoJun 18, 2026
Merged
Conversation
… endpoints Several network-admin AJAX endpoints were registered on wp_ajax_* with no capability check, so any authenticated user (including a subscriber on a sub-site) could reach them. None of these are wired to customer-facing UI; they all back network-admin tools. This enforces manage_network on: - Ajax::search_models / search_all_models — returned network-wide objects and, for the 'user' model, WordPress logins and email addresses (user/email enumeration). - View_Logs_Admin_Page::handle_view_logs — also replaces the substring "is it under the logs folder?" check with realpath() containment so a crafted path can no longer traverse out of the logs directory and read arbitrary files (e.g. wp-config.php). - System_Info_Admin_Page::generate_text_file_system_info — system report. - Dashboard_Widgets::process_ajax_fetch_rss — also pins the outbound feed URL to the plugin's own community feed (filterable) so the endpoint can no longer be used as an SSRF probe; and handle_table_csv. - Domain_Manager::get_dns_records and ::test_integration — DNS lookups and hosting-provider connection tests. - Site_Manager::get_site_screenshot — screenshot scraper. - Template_Placeholders::save_placeholders / serve_placeholders_via_ajax. - Base_Customer_Facing_Admin_Page customize form: capability 'exist' (any logged-in user) raised to 'manage_network'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds Changesmanage_network Capability Hardening and Path/SSRF/Nonce Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
superdav42
approved these changes
Jun 17, 2026
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/site-templates/class-template-placeholders.php (1)
164-179: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winReturn
WP_ErrorJSON failures for authorization and nonce errors.The new authorization branch returns a custom array via
wp_send_json(), which produces a different failure shape from the other hardened endpoints. Usewp_send_json_error(new \WP_Error(...), 403)here; the nonce failure can be aligned at the same time.♻️ Proposed response-shape alignment
if ( ! current_user_can('manage_network')) { - wp_send_json( - [ - 'code' => 'not-enough-permissions', - 'message' => __('You don\'t have permission to alter placeholders.', 'ultimate-multisite'), - ] - ); + wp_send_json_error( + new \WP_Error('not-enough-permissions', __('You don\'t have permission to alter placeholders.', 'ultimate-multisite')), + 403 + ); } if ( ! check_ajax_referer('wu_edit_placeholders_editing', false, false)) { - wp_send_json( - [ - 'code' => 'not-enough-permissions', - 'message' => __('You don\'t have permission to alter placeholders.', 'ultimate-multisite'), - ] - ); + wp_send_json_error( + new \WP_Error('not-enough-permissions', __('You don\'t have permission to alter placeholders.', 'ultimate-multisite')), + 403 + ); }As per coding guidelines, “Use
WP_Errorfor validation/operation failures — not exceptions. Checkis_wp_error()on return values.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/site-templates/class-template-placeholders.php` around lines 164 - 179, The two permission checks in this block (the current_user_can for 'manage_network' permission and the check_ajax_referer for 'wu_edit_placeholders_editing' nonce) are using inconsistent custom array responses via wp_send_json(). Replace both instances with wp_send_json_error(new \WP_Error(...), 403) to maintain consistent error response shapes across all authorization endpoints. For the WP_Error constructor, use an appropriate error code (such as 'not-enough-permissions') as the first parameter and the translated message as the second parameter, ensuring both authorization and nonce failures return the same structured WP_Error format with a 403 status code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inc/managers/class-domain-manager.php`:
- Around line 1229-1231: The get_dns_records() and test_integration() methods in
the Domain_Manager class lack CSRF protection despite having capability checks.
After the existing current_user_can('manage_network') capability check in both
methods, add a check_ajax_referer() call to verify the nonce. Use
wp_send_json_error() with a 403 HTTP status code if the nonce verification
fails. Then update the corresponding JavaScript callers in dns-table.js and
integration-test.js to include the nonce in their AJAX requests by adding the
nonce to the data parameter being sent with each AJAX call.
In `@inc/managers/class-site-manager.php`:
- Around line 542-544: The get_site_screenshot() AJAX handler lacks nonce
verification, making it vulnerable to CSRF attacks. Add a check_ajax_referer()
call after the current_user_can() capability check in the handler to verify the
nonce with the action 'wu_get_screenshot'. On the client-side in
assets/js/screenshot-scraper.js, update the AJAX request data object to include
the nonce by adding a field with the value generated from
wp_create_nonce('wu_get_screenshot') on the server. This ensures that only
legitimate requests from your application can trigger the screenshot
functionality.
---
Outside diff comments:
In `@inc/site-templates/class-template-placeholders.php`:
- Around line 164-179: The two permission checks in this block (the
current_user_can for 'manage_network' permission and the check_ajax_referer for
'wu_edit_placeholders_editing' nonce) are using inconsistent custom array
responses via wp_send_json(). Replace both instances with wp_send_json_error(new
\WP_Error(...), 403) to maintain consistent error response shapes across all
authorization endpoints. For the WP_Error constructor, use an appropriate error
code (such as 'not-enough-permissions') as the first parameter and the
translated message as the second parameter, ensuring both authorization and
nonce failures return the same structured WP_Error format with a 403 status
code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75c463b9-2c6c-41bb-af49-cfef2ff88a07
📒 Files selected for processing (8)
inc/admin-pages/class-base-customer-facing-admin-page.phpinc/admin-pages/class-system-info-admin-page.phpinc/admin-pages/class-view-logs-admin-page.phpinc/class-ajax.phpinc/class-dashboard-widgets.phpinc/managers/class-domain-manager.phpinc/managers/class-site-manager.phpinc/site-templates/class-template-placeholders.php
Collaborator
|
CLAIM_RELEASED reason=worker_complete runner=superdav42 ts=2026-06-17T02:18:36Z aidevops_version=3.20.86 opencode_version=1.17.7 |
Collaborator
Stuck-merge detector: PR has been merge-eligible but unmerged past the thresholdThe pulse merge pass has classified PR #1371 as Failing checks on PR #1371
Worker guidance for the next attempt
Why you're seeing thisEvery pulse cycle (~120s) the deterministic merge pass re-evaluates open PRs. PRs that pass APPROVED + MERGEABLE but fail required checks have historically been re-evaluated silently every cycle until a human noticed. The stuck-merge detector (t3193) surfaces them after Posted automatically by aidevops.sh v3.20.86 automated scan. |
superdav42
added
the
review-feedback-scanned
This was referenced Jun 22, 2026
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
2 participants
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.
Summary
Several
wp_ajax_*endpoints that back network-admin tools were registeredwith no capability check, so any authenticated user (including a subscriber on a
sub-site) could reach them. None of these are wired to customer-facing UI.
This enforces
manage_networkon each, and adds two endpoint-specific hardenings.Changes
Ajax::search_models/search_all_models— returned network-wide objectsand, for the
usermodel, WordPress logins and email addresses(user/email enumeration). Restricted to network admins.
View_Logs_Admin_Page::handle_view_logs— capability check and the"is this under the logs folder?" substring test replaced with
realpath()containment so a crafted path can no longer traverse out of the logs
directory (arbitrary file read).
Dashboard_Widgets::process_ajax_fetch_rss— capability check and theoutbound feed URL is now pinned to the plugin's own community feed
(filterable), removing an SSRF vector; plus
handle_table_csv.System_Info_Admin_Page::generate_text_file_system_info— system report.Domain_Manager::get_dns_records/test_integration— DNS lookups andhosting-provider connection tests.
Site_Manager::get_site_screenshot— screenshot scraper.Template_Placeholders::save_placeholders/serve_placeholders_via_ajax.Base_Customer_Facing_Admin_Pagecustomize form: capabilityexistraised to
manage_network.Compatibility
These endpoints are only ever invoked from network-admin screens, so legitimate
use is unaffected. The customer-facing DNS flow uses a different action
(
wu_get_dns_records_for_domain) and is not touched.Part of a small series of focused security hardening PRs. Full technical detail
is available privately to the maintainers on request (coordinated disclosure).
Summary by CodeRabbit
manage_network, returning HTTP 403 when unauthorized.window.