Skip to content

fix: relax over-strict validation_rules() so abilities accept minimal input #755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
superdav42 merged 1 commit into from
Apr 7, 2026
Merged

fix: relax over-strict validation_rules() so abilities accept minimal input #755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions inc/functions/product.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,25 +116,29 @@ function wu_get_product_by($column, $value) {
*/
function wu_create_product($product_data) {

// Note: empty-string sentinels (instead of `false`) for fields that
// have model-level `default:` rules. rakit's validator treats `false`
// as a non-empty value (`!is_null(false)` is true), so model defaults
// would never fire if we used `false` here.
$product_data = wp_parse_args(
$product_data,
[
'name' => false,
'description' => false,
'currency' => false,
'pricing_type' => false,
'setup_fee' => false,
'name' => '',
'description' => '',
'currency' => '',
'pricing_type' => '',
'setup_fee' => '',
'parent_id' => 0,
'slug' => false,
'recurring' => false,
'slug' => '',
'recurring' => '',
'trial_duration' => 0,
'trial_duration_unit' => 'day',
'duration' => 1,
'duration_unit' => 'day',
'amount' => false,
'billing_cycles' => false,
'active' => false,
'type' => false,
'amount' => '',
'billing_cycles' => '',
'active' => '',
'type' => '',
'featured_image_id' => 0,
'list_order' => 0,
'date_created' => wu_get_current_time('mysql', true),
Expand Down
21 changes: 20 additions & 1 deletion inc/functions/site.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,29 @@ function wu_create_site($site_data) {

$current_site = get_current_site();

$network_domain = $current_site->domain;

// Mode-aware domain/path normalisation. When the caller passes a path
// but no explicit domain (or only the network root domain) on a
// subdomain multisite install, the path is meaningless — WordPress
// won't route to it. Convert the supplied path into a subdomain
// prefix instead so the site is reachable. Callers that pass an
// explicit non-root domain are respected as-is so subdomain/subdir
// mixing still works.
$path_supplied = isset($site_data['path']) && '' !== $site_data['path'] && '/' !== $site_data['path'];
$domain_supplied = isset($site_data['domain']) && '' !== $site_data['domain'] && $site_data['domain'] !== $network_domain;

if (is_multisite() && is_subdomain_install() && $path_supplied && ! $domain_supplied) {
Comment on lines +241 to +243
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Normalize root-domain comparison before setting domain_supplied

Line 241 can misclassify a root domain as “explicit non-root” when forms differ (www.example.com vs example.com, or case differences), which skips the subdomain conversion path unexpectedly.

Suggested patch 
-	$domain_supplied = isset($site_data['domain']) && '' !== $site_data['domain'] && $site_data['domain'] !== $network_domain;
+	$normalized_network_domain = strtolower((string) preg_replace('/^www\./i', '', (string) $network_domain));
+	$normalized_input_domain   = isset($site_data['domain'])
+		? strtolower((string) preg_replace('/^www\./i', '', (string) $site_data['domain']))
+		: '';
+	$domain_supplied = '' !== $normalized_input_domain && $normalized_input_domain !== $normalized_network_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.

Suggested change
$domain_supplied = isset($site_data['domain']) && '' !== $site_data['domain'] && $site_data['domain'] !== $network_domain;
if (is_multisite() && is_subdomain_install() && $path_supplied && ! $domain_supplied) {
$normalized_network_domain = strtolower((string) preg_replace('/^www\./i', '', (string) $network_domain));
$normalized_input_domain = isset($site_data['domain'])
? strtolower((string) preg_replace('/^www\./i', '', (string) $site_data['domain']))
: '';
$domain_supplied = '' !== $normalized_input_domain && $normalized_input_domain !== $normalized_network_domain;
if (is_multisite() && is_subdomain_install() && $path_supplied && ! $domain_supplied) {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@inc/functions/site.php` around lines 241 - 243, The comparison that sets
$domain_supplied can misclassify root vs supplied domains due to case and www
differences; normalize both $site_data['domain'] and $network_domain (lowercase,
strip leading "www.", trim trailing dots/spaces) before comparing so
$domain_supplied = isset($site_data['domain']) && '' !== $site_data['domain'] &&
normalize($site_data['domain']) !== normalize($network_domain); this preserves
the existing logic used later (is_multisite(), is_subdomain_install(),
$path_supplied) while ensuring subdomain conversion runs correctly when domains
differ only by www/casing.

$slug = trim((string) $site_data['path'], '/');
$bare_network_domain = preg_replace('/^www\./i', '', (string) $network_domain);
$site_data['domain'] = "{$slug}.{$bare_network_domain}";
$site_data['path'] = '/';
Comment on lines +244 to +247
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Sanitize and validate the derived subdomain slug before building domain

Line 244 currently trims slashes only, then interpolates directly into a hostname on Line 246. Invalid characters or malformed path values can generate invalid domains. Sanitize the slug and return WP_Error when it collapses to empty.

Suggested patch 
 	if (is_multisite() && is_subdomain_install() && $path_supplied && ! $domain_supplied) {
-		$slug                 = trim((string) $site_data['path'], '/');
+		$raw_slug             = trim((string) $site_data['path'], '/');
+		$slug                 = sanitize_title_with_dashes(wu_clean($raw_slug));
+		if ('' === $slug) {
+			return new \WP_Error(
+				'invalid_site_path',
+				__('Invalid site path for subdomain creation.', 'ultimate-multisite')
+			);
+		}
 		$bare_network_domain  = preg_replace('/^www\./i', '', (string) $network_domain);
 		$site_data['domain']  = "{$slug}.{$bare_network_domain}";
 		$site_data['path']    = '/';
 	}

As per coding guidelines, "Use wu_clean() or WordPress sanitization functions for input sanitization" and "Use WP_Error for validation/operation failures instead of exceptions".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@inc/functions/site.php` around lines 244 - 247, The code currently builds
$slug from $site_data['path'] then directly interpolates it into
$site_data['domain' ], which can produce invalid hostnames; update the logic
that sets $slug (derived from $site_data['path']) to sanitize it using the
project's sanitizer (e.g., wu_clean() or a WP sanitization helper such as
sanitize_title/sanitize_key) and normalize/remap invalid chars, then validate
that the sanitized slug is not empty; if it is empty or invalid, return a
WP_Error instead of continuing. Apply these changes where $slug is declared/used
and ensure $site_data['domain'] is only set after successful
sanitization/validation.

}

$site_data = wp_parse_args(
$site_data,
[
'domain' => $current_site->domain,
'domain' => $network_domain,
'path' => '/',
'title' => false,
'type' => false,
Expand Down
3 changes: 2 additions & 1 deletion inc/models/class-broadcast.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ public function validation_rules() {
'name' => 'default:title',
'title' => 'required|min:2',
'content' => 'required|min:3',
'type' => 'required|in:broadcast_email,broadcast_notice|default:broadcast_notice',
// Has a default — `required` is redundant.
'type' => 'in:broadcast_email,broadcast_notice|default:broadcast_notice',
];
}

Expand Down
7 changes: 5 additions & 2 deletions inc/models/class-customer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ public function validation_rules() {

return [
'user_id' => "required|integer|unique:\WP_Ultimo\Models\Customer,user_id,{$id}",
'email_verification' => 'required|in:none,pending,verified',
'type' => 'required|in:customer',
// Defaults to "none" so callers don't have to choose a verification
// state on creation — they can update it later if needed.
'email_verification' => 'in:none,pending,verified|default:none',
// Currently the only valid value, so default it.
'type' => 'in:customer|default:customer',
'last_login' => 'default:',
'has_trialed' => 'boolean|default:0',
'vip' => 'boolean|default:0',
Expand Down
3 changes: 2 additions & 1 deletion inc/models/class-domain.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ public function validation_rules() {
return [
'blog_id' => 'required|integer',
'domain' => "required|domain|unique:\WP_Ultimo\Models\Domain,domain,{$id}",
'stage' => 'required|in:checking-dns,checking-ssl-cert,done-without-ssl,done,failed,ssl-failed|default:checking-dns',
// Has a default — `required` is redundant.
'stage' => 'in:checking-dns,checking-ssl-cert,done-without-ssl,done,failed,ssl-failed|default:checking-dns',
'active' => 'default:1',
'secure' => 'default:0',
'primary_domain' => 'default:0',
Expand Down
4 changes: 3 additions & 1 deletion inc/models/class-payment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,9 @@ public function validation_rules(): array {
'tax_total' => 'numeric',
'discount_code' => 'alpha_dash',
'total' => 'required|numeric',
'status' => "required|in:{$payment_types}",
// Defaults to "pending" so callers can record a payment without
// having to choose a status up front; gateways update it later.
'status' => "in:{$payment_types}|default:pending",
'gateway' => 'default:',
'gateway_payment_id' => 'default:',
'discount_total' => 'integer',
Expand Down
10 changes: 7 additions & 3 deletions inc/models/class-product.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,11 @@ public function validation_rules() {

return [
'featured_image_id' => 'integer',
'currency' => "required|default:{$currency}",
'pricing_type' => 'required|in:free,paid,contact_us,pay_what_you_want',
// Has a default — `required` is redundant.
'currency' => "default:{$currency}",
// Defaults to "free" so a caller can create a basic product
// without having to choose a pricing model up front.
'pricing_type' => 'in:free,paid,contact_us,pay_what_you_want|default:free',
'trial_duration' => 'integer',
'trial_duration_unit' => 'in:day,week,month,year|default:month',
'parent_id' => 'integer',
Expand All @@ -430,7 +433,8 @@ public function validation_rules() {
'billing_cycles' => 'integer|default:0',
'active' => 'default:1',
'price_variations' => "price_variations:{$duration},{$duration_unit}",
'type' => "required|default:plan|in:{$allowed_types}",
// Has a default — `required` is redundant.
'type' => "default:plan|in:{$allowed_types}",
'slug' => "required|unique:\WP_Ultimo\Models\Product,slug,{$id}|min:2",
'taxable' => 'boolean|default:0',
'tax_category' => 'default:',
Expand Down
21 changes: 15 additions & 6 deletions inc/models/class-site.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,10 +331,15 @@ public function validation_rules() {
return [
'categories' => 'default:',
'featured_image_id' => 'integer|default:',
'site_id' => 'required|integer',
// site_id is the network id; defaults to 1 (the main network) in
// the model, so it's optional for a regular WordPress subsite.
'site_id' => 'integer|default:1',
Comment on lines +334 to +336
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Defaulting site_id to 1 can create sites in the wrong network.

Line 336 hard-defaults to 1, and save() later uses $this->get_site_id() ?: get_current_network_id(), so omitted site_id never falls back to the current network. On multi-network installs, this can route creations to network 1 unintentionally.

Suggested fix 
-			'site_id'           => 'integer|default:1',
+			'site_id'           => 'integer|default:',
-	protected $site_id = 1;
+	protected $site_id = 0;
📝 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
// site_id is the network id; defaults to 1 (the main network) in
// the model, so it's optional for a regular WordPress subsite.
'site_id' => 'integer|default:1',
// site_id is the network id; defaults to 1 (the main network) in
// the model, so it's optional for a regular WordPress subsite.
'site_id' => 'integer|default:',
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@inc/models/class-site.php` around lines 334 - 336, The schema entry for
'site_id' currently hard-defaults to 1 causing creations to always use network
1; remove that hard-default (or change it to allow null) so the validator rule
is just 'integer' (or 'integer|nullable') and let the existing save() logic
(which calls $this->get_site_id() ?: get_current_network_id()) determine the
correct network; update the 'site_id' rule in the class-site.php model instead
of defaulting to 1 to avoid forcing network 1 on multi-network installs.

'title' => 'required',
'name' => 'required',
'description' => 'required|min:2',
// description is optional metadata. A regular WordPress subsite
// doesn't have one at the network level — it lives in
// blogdescription per blog and can be set later.
'description' => 'default:',
'domain' => 'domain',
'path' => 'required|default:',
'registered' => "default:{$date}",
Expand All @@ -346,8 +351,12 @@ public function validation_rules() {
'deleted' => 'boolean|default:0',
'is_publishing' => 'boolean|default:0',
'land_id' => 'integer|default:',
'customer_id' => 'required|integer|exists:\WP_Ultimo\Models\Customer,id',
'membership_id' => 'required|integer|exists:\WP_Ultimo\Models\Membership,id',
// customer_id and membership_id are WaaS-specific. A vanilla
// WordPress subsite (type=default) does not need a paying
// customer or a membership. Keep the foreign-key existence
// check so customer-owned sites still validate when supplied.
'customer_id' => 'integer|default:|exists:\WP_Ultimo\Models\Customer,id',
'membership_id' => 'integer|default:|exists:\WP_Ultimo\Models\Membership,id',
'template_id' => 'integer|default:',
'type' => "required|in:{$site_types}",
'signup_options' => 'default:',
Expand Down Expand Up @@ -631,7 +640,7 @@ public function get_domain() {
* Set domain name used by this site..
*
* @since 2.0.0
* @param string $domain The site domain. You don't need to put http or https in front of your domain in this field. e.g: example.com.
* @param string $domain Full hostname for the site, no protocol. On a subdomain multisite install supply the full subdomain you want (e.g. "blog.example.com"). On a subdirectory install leave empty to inherit the network root domain. wu_create_site() will auto-derive this from `path` on subdomain installs when you only supply a slug.
* @return void
*/
public function set_domain($domain): void {
Expand All @@ -653,7 +662,7 @@ public function get_path(): string {
* Set path of the site. Used when in sub-directory mode..
*
* @since 2.0.0
* @param string $path Path of the site. Used when in sub-directory mode.
* @param string $path URL path for the site. On a subdirectory multisite install this is the URL prefix (e.g. "/blog/"). On a subdomain install supply just the slug (e.g. "blog") and wu_create_site() will convert it into the full subdomain "blog.<network-domain>" automatically.
* @return void
*/
public function set_path($path): void {
Expand Down
5 changes: 4 additions & 1 deletion inc/models/class-webhook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,10 @@ public function validation_rules() {
'event_count' => 'default:0',
'active' => 'default:1',
'hidden' => 'default:0',
'integration' => 'required|min:2',
// Defaults to "manual" so a caller can register a webhook
// without specifying which integration owns it. Plugins that
// register webhooks set their own integration tag.
'integration' => 'min:2|default:manual',
'date_last_failed' => 'default:',
];
}
Expand Down
Loading