Reverse proxy fix #191
Reverse proxy fix #191
Conversation
WalkthroughAdds loopback-based domain verification and an async DNS mapping endpoint, introduces lazy-loading for Limitations modules, guards against overwriting existing user roles on sites, adjusts domain lookup behavior, updates sanitization and log rendering to safer methods, and refreshes a README badge. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant S as Site/Domain Logic
participant DM as Domain_Manager
participant D as Domain (model)
participant HTTP as Loopback Request
S->>D: has_correct_dns()
D->>DM: verify_domain_with_loopback_request(domain)
rect rgba(200,240,255,0.3)
DM->>HTTP: GET https://domain/?nonce=... (timeout 10s)
alt 200 OK JSON and id matches
HTTP-->>DM: {"id": <domain_id>, ...}
DM-->>D: true
D-->>S: true (short-circuit)
else fallback to http or errors
DM->>HTTP: GET http://domain/?nonce=...
alt success with matching id
HTTP-->>DM: {"id": <domain_id>}
DM-->>D: true
D-->>S: true
else all attempts fail
DM-->>D: false
D->>S: continue DNS/IP checks
end
end
end
sequenceDiagram
autonumber
participant R as Request (ms_site_not_found)
participant DMAP as Domain_Mapping
participant D as Domain (model)
participant WP as WP Core
R->>DMAP: verify_dns_mapping($current_site, $domain, $path)
alt async_check_dns_nonce present
DMAP->>WP: Load pluggable + formatting (nonce/hash)
DMAP->>DMAP: hash_equals( wp_hash($domain), nonce )
alt nonce valid
DMAP->>D: get_by_domain([naked, www])
alt mapping found
D-->>DMAP: mapping
DMAP-->>R: wp_send_json(mapping)
else not found
DMAP-->>R: no output (continue)
end
else invalid nonce
DMAP-->>R: no output (continue)
end
else no nonce
DMAP-->>R: no output (continue)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
views/admin-notices.php (1)
34-34: Escape URLs with esc_url, not esc_attr (security).href should use esc_url to enforce allowed protocols.
Apply this diff:
- <a class="wu-bg-white ... " title="<?php echo esc_attr($action['title']); ?>" href="<?php echo esc_attr($action['url']); ?>"><?php echo wp_kses_post($action['title']); ?></a> + <a class="wu-bg-white ... " title="<?php echo esc_attr($action['title']); ?>" href="<?php echo esc_url($action['url']); ?>"><?php echo wp_kses_post($action['title']); ?></a>
🧹 Nitpick comments (12)
README.md (1)
21-21: Badge repo switch looks good; consider aligning other repo links.Many links still point to superdav42/wp-multisite-waas; update for consistency when convenient.
inc/objects/class-limitations.php (3)
86-90: Lazy-build is fine; add a defensive default.Guard against missing/invalid entries to avoid edge notices.
Apply this diff:
- $module = self::build($this->raw_module_data[ $name ] ?? [], $name); + $data = isset($this->raw_module_data[$name]) ? $this->raw_module_data[$name] : []; + $module = self::build($data, $name);
181-188: Prevent undefined index notices in has_limitations().Some modules may omit 'enabled'. Use a safe coalesce.
Apply this diff:
- foreach ($this->raw_module_data as $module) { - if ($module['enabled']) { - return true; - } - } - return false; + foreach ($this->raw_module_data as $module) { + if (is_array($module) && ! empty($module['enabled'])) { + return true; + } + } + return false;
200-201: is_module_enabled() is fine; optional stricter cast.Consider (bool) for clarity.
Apply this diff:
- return $this->raw_module_data[ $module_name ]['enabled'] ?? false; + return (bool) ($this->raw_module_data[$module_name]['enabled'] ?? false);tests/WP_Ultimo/Objects/Limitations_Test.php (3)
79-86: Avoid coupling tests to internals: prefer public API over reflection of raw_module_dataAsserting via the protected raw_module_data makes tests brittle. Use the public to_array() instead.
Apply:
-// Use reflection to access protected modules property -$reflection = new \ReflectionClass($limitations); -$modules_property = $reflection->getProperty('raw_module_data'); -$modules_property->setAccessible(true); -$modules = $modules_property->getValue($limitations); +$modules = $limitations->to_array();
200-207: Same here: stop reflecting raw_module_data; assert via to_array()Keeps tests resilient to future refactors.
Apply:
-// Use reflection to access protected modules property -$reflection = new \ReflectionClass($limitations); -$modules_property = $reflection->getProperty('raw_module_data'); -$modules_property->setAccessible(true); -$modules = $modules_property->getValue($limitations); +$modules = $limitations->to_array();
287-292: Redundant assertion; tighten the checkassertNotFalse + assertTrue is redundant; also assert the type.
Apply:
- if ($should_exist) { - $this->assertNotFalse($result); - $this->assertTrue($result); - } else { + if ($should_exist) { + $this->assertIsBool($result); + $this->assertTrue($result); + } else { $this->assertFalse($result); }views/domain/dns-table.php (1)
78-81: Render human-friendly Yes/No and handle undefined stateBooleans render as “true/false”. Show localized Yes/No; optionally guard for undefined.
Apply:
- <tr> - <td colspan="2"><?php esc_html_e('Domain connected to site?', 'multisite-ultimate'); ?></td> - <td colspan="2" class="wu-text-left">{{ results.has_correct_dns }}</td> - </tr> + <tr> + <td colspan="2"><?php esc_html_e('Domain connected to site?', 'multisite-ultimate'); ?></td> + <td colspan="2" class="wu-text-left"> + <span v-if="results && results.has_correct_dns === true"><?php esc_html_e('Yes', 'multisite-ultimate'); ?></span> + <span v-else-if="results && results.has_correct_dns === false"><?php esc_html_e('No', 'multisite-ultimate'); ?></span> + <span v-else>—</span> + </td> + </tr>Please confirm that the frontend payload feeding this template includes results.has_correct_dns. If not, we should add it where the DNS data is assembled.
inc/models/class-domain.php (1)
400-410: Early short-circuit via secret verification looks goodNicely preserves the DNS path while enabling Cloudflare-friendly verification behind a filter.
Minor: drop the unused local variable.
- $domain_manager = \WP_Ultimo\Managers\Domain_Manager::get_instance(); - $secret_verification_enabled = apply_filters('wu_enable_secret_domain_verification', true, $this); + $secret_verification_enabled = apply_filters('wu_enable_secret_domain_verification', true, $this); if ( $secret_verification_enabled ) { - $secret_result = $domain_manager->verify_domain_with_secret($domain_url); + $secret_result = \WP_Ultimo\Managers\Domain_Manager::get_instance()->verify_domain_with_secret($domain_url); if ( $secret_result ) { return true; } }inc/managers/class-domain-manager.php (3)
1027-1037: Harden JSON parsing and shape checksGuard against invalid/HTML responses from proxies or WAFs.
Apply:
- $body = wp_remote_retrieve_body($response); - $data = json_decode($body, true); + $body = wp_remote_retrieve_body($response); + $data = json_decode($body, true); + if (!is_array($data)) { + wu_log_add( + "domain-verification-{$domain}", + __('Verification endpoint returned invalid JSON.', 'multisite-ultimate'), + LogLevel::WARNING + ); + return false; + }
914-931: Tighten REST arg validation for domain parameterAdd validate_callback; also consider making TTL filterable.
Apply:
- 'args' => array( - 'domain' => array( - 'required' => true, - 'sanitize_callback' => 'sanitize_text_field', - ), - ), + 'args' => array( + 'domain' => array( + 'required' => true, + 'sanitize_callback' => 'sanitize_text_field', + 'validate_callback' => function ($param) { + return (bool) filter_var($param, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME); + }, + ), + ),And in generate_domain_verification_secret, make TTL filterable:
- set_site_transient("wu_domain_verification_{$domain}", $secret, 5 * MINUTE_IN_SECONDS); + set_site_transient( + "wu_domain_verification_{$domain}", + $secret, + (int) apply_filters('wu_domain_verification_ttl', 5 * MINUTE_IN_SECONDS, $domain) + );
967-974: Optional: use cryptographically strong generatorwp_generate_password is fine here, but random_bytes(16) + bin2hex() is stronger.
Apply:
- $secret = wp_generate_password(32, false); + $secret = bin2hex(random_bytes(16));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
README.md(1 hunks)inc/limits/class-customer-user-role-limits.php(1 hunks)inc/managers/class-domain-manager.php(1 hunks)inc/models/class-domain.php(1 hunks)inc/models/class-site.php(1 hunks)inc/objects/class-limitations.php(7 hunks)tests/WP_Ultimo/Objects/Limitations_Test.php(3 hunks)views/admin-notices.php(1 hunks)views/domain/dns-table.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
inc/models/class-domain.php (1)
inc/managers/class-domain-manager.php (1)
verify_domain_with_secret(1003-1056)
inc/managers/class-domain-manager.php (1)
inc/functions/helper.php (1)
wu_log_add(208-211)
inc/limits/class-customer-user-role-limits.php (2)
inc/models/class-customer.php (1)
get_user_id(167-170)inc/models/class-site.php (1)
get_id(451-454)
inc/objects/class-limitations.php (1)
inc/functions/helper.php (1)
wu_get_isset(66-73)
tests/WP_Ultimo/Objects/Limitations_Test.php (1)
inc/objects/class-limitations.php (1)
exists(168-171)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.3, firefox)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (8.0, chrome)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (7.4, chrome)
🔇 Additional comments (6)
views/admin-notices.php (1)
21-21: Consistent sanitization with wp_kses_post is OK.Aligns both branches; safe for limited HTML in messages.
inc/objects/class-limitations.php (5)
70-71: Constructor storing raw_module_data looks good.
118-119: Unserialize into raw store is OK.
170-171: exists() now checks raw data key presence — good.
359-360: No direct array_merge calls on Limitations::to_array(); only used in wu_array_recursive_diff, which handles raw arrays as expected.
131-134: build_modules safe: no eager callers
build_modules is only invoked in tests (asserting it returns self); no production code calls it or relies on pre-instantiated modules.
| // only add user to blog if they are not already a member, or we are downgrading their role. | ||
| // Without this check the user could lose additional roles added manually or with hooks. | ||
| if ('administrator' !== $role || ! is_user_member_of_blog($customer->get_user_id(), $site->get_id())) { | ||
| add_user_to_blog($site->get_id(), $customer->get_user_id(), $role); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid role update when customer has no WP user_id.
Prevent accidental calls with a falsy user_id; minor perf win by not repeating get_user_id().
Apply this diff:
- // only add user to blog if they are not already a member, or we are downgrading their role.
- // Without this check the user could lose additional roles added manually or with hooks.
- if ('administrator' !== $role || ! is_user_member_of_blog($customer->get_user_id(), $site->get_id())) {
- add_user_to_blog($site->get_id(), $customer->get_user_id(), $role);
- }
+ // only add user to blog if they are not already a member, or we are downgrading their role.
+ // Without this check the user could lose additional roles added manually or with hooks.
+ $uid = (int) $customer->get_user_id();
+ if ($uid && ('administrator' !== $role || ! is_user_member_of_blog($uid, $site->get_id()))) {
+ add_user_to_blog($site->get_id(), $uid, $role);
+ }📝 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.
| // only add user to blog if they are not already a member, or we are downgrading their role. | |
| // Without this check the user could lose additional roles added manually or with hooks. | |
| if ('administrator' !== $role || ! is_user_member_of_blog($customer->get_user_id(), $site->get_id())) { | |
| add_user_to_blog($site->get_id(), $customer->get_user_id(), $role); | |
| } | |
| // only add user to blog if they are not already a member, or we are downgrading their role. | |
| // Without this check the user could lose additional roles added manually or with hooks. | |
| $uid = (int) $customer->get_user_id(); | |
| if ( $uid && ( 'administrator' !== $role || ! is_user_member_of_blog( $uid, $site->get_id() ) ) ) { | |
| add_user_to_blog( $site->get_id(), $uid, $role ); | |
| } |
🤖 Prompt for AI Agents
In inc/limits/class-customer-user-role-limits.php around lines 138 to 142, avoid
calling add_user_to_blog or is_user_member_of_blog when the customer has no WP
user_id and avoid repeated get_user_id() calls: retrieve the customer user_id
once into a local variable, return/skip early if it's falsy, then perform the
existing check using that cached user_id and only call add_user_to_blog when
appropriate.
|
|
||
| /** | ||
| * Register the domain verification endpoint. | ||
| * | ||
| * @since 2.0.0 | ||
| * @return void | ||
| */ | ||
| public function register_domain_verification_endpoint(): void { | ||
|
|
||
| register_rest_route( | ||
| 'wu/v1', | ||
| '/domain-verification/(?P<domain>[a-zA-Z0-9.-]+)', | ||
| array( | ||
| 'methods' => \WP_REST_Server::READABLE, | ||
| 'callback' => array($this, 'domain_verification_endpoint'), | ||
| 'permission_callback' => '__return_true', | ||
| 'args' => array( | ||
| 'domain' => array( | ||
| 'required' => true, | ||
| 'sanitize_callback' => 'sanitize_text_field', | ||
| ), | ||
| ), | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure the endpoint is actually registered
I don’t see a hook here. If the Rest_Api trait doesn’t auto-register this, add a rest_api_init hook.
Add in init():
public function init(): void {
$this->enable_rest_api();
+ add_action('rest_api_init', [$this, 'register_domain_verification_endpoint']);
$this->enable_wp_cli();To verify:
🏁 Script executed:
#!/bin/bash
# Confirm route is registered and where it's hooked
rg -nP -C2 "register_rest_route\(\s*['\"]wu\/v1['\"].*domain-verification"
rg -n "rest_api_init" -C2Length of output: 1102
Register your endpoint on rest_api_init
The register_domain_verification_endpoint() method exists but isn’t hooked anywhere—neither via your init() nor by the Rest_Api trait’s register_routes_* methods. You must add:
public function init(): void {
$this->enable_rest_api();
+ add_action('rest_api_init', [$this, 'register_domain_verification_endpoint']);
$this->enable_wp_cli();
}📝 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.
| /** | |
| * Register the domain verification endpoint. | |
| * | |
| * @since 2.0.0 | |
| * @return void | |
| */ | |
| public function register_domain_verification_endpoint(): void { | |
| register_rest_route( | |
| 'wu/v1', | |
| '/domain-verification/(?P<domain>[a-zA-Z0-9.-]+)', | |
| array( | |
| 'methods' => \WP_REST_Server::READABLE, | |
| 'callback' => array($this, 'domain_verification_endpoint'), | |
| 'permission_callback' => '__return_true', | |
| 'args' => array( | |
| 'domain' => array( | |
| 'required' => true, | |
| 'sanitize_callback' => 'sanitize_text_field', | |
| ), | |
| ), | |
| ) | |
| ); | |
| } | |
| public function init(): void { | |
| $this->enable_rest_api(); | |
| add_action( 'rest_api_init', [ $this, 'register_domain_verification_endpoint' ] ); | |
| $this->enable_wp_cli(); | |
| } |
🤖 Prompt for AI Agents
In inc/managers/class-domain-manager.php around lines 907 to 931, the
register_domain_verification_endpoint() method is defined but never hooked into
WordPress so the route is never registered; wire this method into the REST
bootstrap by hooking it to rest_api_init (or by calling it from the class init()
or the Rest_Api trait's route registration flow) so the endpoint is registered
at runtime—add the appropriate add_action('rest_api_init', ...) registration or
invoke the method from the existing route registration entry point to ensure the
route is registered.
| public function verify_domain_with_secret($domain) { | ||
|
|
||
| $expected_secret = $this->generate_domain_verification_secret($domain); | ||
|
|
||
| $verification_url = "https://{$domain}/wp-json/wu/v1/domain-verification/{$domain}"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid regenerating the secret on each verification attempt
Regenerating can cause race conditions; fetch the current secret instead.
Apply:
- $expected_secret = $this->generate_domain_verification_secret($domain);
+ $expected_secret = $this->get_domain_verification_secret($domain);📝 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.
| public function verify_domain_with_secret($domain) { | |
| $expected_secret = $this->generate_domain_verification_secret($domain); | |
| $verification_url = "https://{$domain}/wp-json/wu/v1/domain-verification/{$domain}"; | |
| public function verify_domain_with_secret($domain) { | |
| $expected_secret = $this->get_domain_verification_secret($domain); | |
| $verification_url = "https://{$domain}/wp-json/wu/v1/domain-verification/{$domain}"; |
🤖 Prompt for AI Agents
In inc/managers/class-domain-manager.php around lines 1003 to 1007, do not call
generate_domain_verification_secret($domain) during verification because it
re-creates the secret and can cause race conditions; instead retrieve the
already-stored/current secret (e.g. from the option/metadata or a dedicated
getter like get_stored_domain_verification_secret($domain)) and compare that
stored value to the secret presented during verification, falling back to a safe
behaviour if no stored secret exists; update the verification logic to use the
fetched secret and remove the regeneration call.
| $verification_url, | ||
| array( | ||
| 'timeout' => 10, | ||
| 'sslverify' => false, | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Do not disable TLS verification on remote requests
Disabling sslverify invites MITM and weakens transport security.
Apply:
- 'timeout' => 10,
- 'sslverify' => false,
+ 'timeout' => 10,
+ 'sslverify' => apply_filters('wu_verify_domain_secret_sslverify', true),
+ 'redirection' => 3,
+ 'headers' => array('Accept' => 'application/json'),📝 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.
| $verification_url, | |
| array( | |
| 'timeout' => 10, | |
| 'sslverify' => false, | |
| ) | |
| ); | |
| $verification_url, | |
| array( | |
| - 'timeout' => 10, | |
| 'timeout' => 10, | |
| 'sslverify' => apply_filters( 'wu_verify_domain_secret_sslverify', true ), | |
| 'redirection' => 3, | |
| 'headers' => array( | |
| 'Accept' => 'application/json', | |
| ), | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In inc/managers/class-domain-manager.php around lines 1010 to 1015, the call to
wp_remote_get currently disables TLS verification by setting 'sslverify' =>
false; remove that option or set 'sslverify' => true so remote requests use
proper certificate validation, and handle valid certificate errors explicitly
(e.g. catch wp_remote_get errors and log/return a clear error if verification
fails, or allow specifying a CA bundle if you must trust a custom CA). Ensure
you do not ship code that disables TLS verification and add a comment noting why
sslverify must remain true.
| // only add user to blog if they are not already a member, or we are downgrading their role. | ||
| // Without this check the user could lose additional roles added manually or with hooks. | ||
| if ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id())) { | ||
| add_user_to_blog($this->get_id(), $user_id, $role); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard add_user_to_blog when user_id is empty.
If get_user_id() returns 0/false, add_user_to_blog may misbehave. Add a truthy check.
Apply this diff:
- if ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id())) {
- add_user_to_blog($this->get_id(), $user_id, $role);
- }
+ if ($user_id && ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id()))) {
+ add_user_to_blog($this->get_id(), $user_id, $role);
+ }📝 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.
| // only add user to blog if they are not already a member, or we are downgrading their role. | |
| // Without this check the user could lose additional roles added manually or with hooks. | |
| if ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id())) { | |
| add_user_to_blog($this->get_id(), $user_id, $role); | |
| } | |
| // only add user to blog if they are not already a member, or we are downgrading their role. | |
| // Without this check the user could lose additional roles added manually or with hooks. | |
| if ( $user_id && ( 'administrator' !== $role || ! is_user_member_of_blog( $user_id, $this->get_id() ) ) ) { | |
| add_user_to_blog( $this->get_id(), $user_id, $role ); | |
| } |
🤖 Prompt for AI Agents
In inc/models/class-site.php around lines 1682 to 1686, add a truthy check for
$user_id before calling is_user_member_of_blog() or add_user_to_blog(), because
get_user_id() can return 0/false; modify the conditional to first ensure
$user_id is truthy (e.g. $user_id) and then perform the existing role/member
checks so add_user_to_blog is only invoked when a valid user ID exists.
…ork for api when the domain is not yet active.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
assets/js/domain-logs.js (1)
15-15: Safer log rendering via text(); preserve formatting and add null-safetyGood XSS hardening. To avoid collapsing newlines and to guard missing data:
Apply:
-$('#content').text(response.data.contents); +$('#content').text((response && response.data && typeof response.data.contents === 'string') ? response.data.contents : '');Optionally ensure CSS preserves line breaks:
#content { white-space: pre-wrap; font-family: monospace; }inc/models/class-domain.php (1)
609-609: Docblock return type should include nullMethod returns an instance or null; reflect that to aid static analysis.
- * @return static + * @return static|nullinc/class-domain-mapping.php (1)
245-262: Harden async verification response (no-cache, consistent shape, unused params)
- Prevent proxies from caching the JSON.
- Use wp_send_json_success for a consistent payload.
- Mark unused parameters to satisfy static analyzers.
-public function verify_dns_mapping($current_site, $domain, $path) { +public function verify_dns_mapping($_current_site, $domain, $_path) { // Nonce functions are unavailable and the wp_hash is basically the same. if (isset($_REQUEST['async_check_dns_nonce'])) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended // This is very early in the request we need these to use wp_hash. require_once ABSPATH . WPINC . '/l10n.php'; require_once ABSPATH . WPINC . '/pluggable.php'; - if (hash_equals(wp_hash($domain), $_REQUEST['async_check_dns_nonce'])) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash + $nonce = (string) $_REQUEST['async_check_dns_nonce']; // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash + if (hash_equals(wp_hash($domain), $nonce)) { $domains = $this->get_www_and_nowww_versions($domain); $mapping = Domain::get_by_domain($domains); if ($mapping) { - wp_send_json($mapping->to_array()); + nocache_headers(); + wp_send_json_success($mapping->to_array()); } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
assets/js/domain-logs.js(1 hunks)inc/class-domain-mapping.php(2 hunks)inc/managers/class-domain-manager.php(2 hunks)inc/models/class-domain.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- inc/managers/class-domain-manager.php
🧰 Additional context used
🧬 Code graph analysis (2)
inc/class-domain-mapping.php (2)
inc/models/class-domain.php (1)
get_by_domain(611-666)inc/models/class-base-model.php (1)
to_array(850-869)
inc/models/class-domain.php (1)
inc/managers/class-domain-manager.php (1)
verify_domain_with_loopback_request(920-1049)
🪛 PHPMD (2.15.0)
inc/class-domain-mapping.php
245-245: Avoid unused parameters such as '$current_site'. (Unused Code Rules)
(UnusedFormalParameter)
245-245: Avoid unused parameters such as '$path'. (Unused Code Rules)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: cypress (8.4, firefox)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, firefox)
- GitHub Check: cypress (8.4, chrome)
- GitHub Check: cypress (8.3, chrome)
- GitHub Check: cypress (8.0, chrome)
- GitHub Check: cypress (8.0, firefox)
- GitHub Check: cypress (8.2, firefox)
- GitHub Check: cypress (7.4, firefox)
- GitHub Check: cypress (7.4, chrome)
🔇 Additional comments (2)
inc/models/class-domain.php (1)
642-642: Removing active = 1 broadens results; confirm all call sites gate on is_active()This will now return inactive mappings (ordered by primary_domain, active, secure). check_domain_mapping() already gates via wu_use_domain_mapping and is_active(), which is good. Please verify other consumers of get_by_domain() don’t assume an active mapping.
inc/class-domain-mapping.php (1)
125-126: Good: early ms_site_not_found hook for async verificationPriority 5 and 3 args are appropriate for an early, lightweight responder.
| $domain_manager = \WP_Ultimo\Managers\Domain_Manager::get_instance(); | ||
|
|
||
| if ($domain_manager->verify_domain_with_loopback_request($this)) { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Loopback short-circuit is fine; ensure TLS verification is actually enforced
This path depends on Domain_Manager::verify_domain_with_loopback_request(). In that method, the “HTTPS with SSL verification” variant currently uses sslverify tied to https_local_ssl_verify with a default false. That disables certificate verification by default and invites MITM.
Proposed fix in inc/managers/class-domain-manager.php:
- 'sslverify' => apply_filters('https_local_ssl_verify', false),
+ 'sslverify' => apply_filters('https_local_ssl_verify', true),Also ensure the log label matches behavior (i.e., only claim “with SSL verification” when sslverify is true).
fix dns verify with cloudflare
Summary by CodeRabbit