Robustness: null-deref, div-by-zero and control-flow guards

[vuckro]

Summary

A bundle of small, isolated robustness fixes found during a reliability review.
Each change is independent and a few lines; grouped for easier review.

• functions/currency.php — add missing break; so BYN no longer falls
  through and renders the CHF symbol.
• functions/discount-code.php — initialise $discounted_price to the base
  price so an unknown discount type fails safe instead of returning an undefined
  value.
• models/class-membership.php — get_normalized_amount() guards against a
  zero duration before dividing.
• models/class-site.php — get_all_by_type('pending') validates the
  unserialized value is a Site before calling ->set_type(), and filters out
  bad rows.
• sso/class-magic-link.php — generate_magic_link() bails when
  wu_get_site() returns false (mirrors sibling methods) instead of
  dereferencing null.
• installers/class-migrator.php — guard $customer (not $membership)
  before $customer->get_id() when building migrated payment data.
• integrations/host-providers/class-cloudflare-host-provider.php — guard the
  zones API result before iterating ->result.
• integrations/host-providers/class-dns-record.php — read the Hestia record
  content key (falling back to value) so record content isn't dropped.

Summary by CodeRabbit

• Bug Fixes
  • Fixed Brazilian Real (BRL) currency symbol handling to avoid incorrect fall-through
  • Prevented discounted price calculations from using an undefined value for unsupported types
  • Hardened payment transaction customer ID assignment to avoid null dereferences
  • Improved Cloudflare DNS zone ID collection robustness against error/empty responses
  • Corrected DNS record content mapping for the Hestia provider
  • Avoided division-by-zero in normalized membership amount calculations
  • Validated and filtered pending site deserialization results
  • Made magic link generation fail safely when the target site is missing

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2193a028-0516-4247-b4f4-107163ffb09d

📥 Commits

Reviewing files that changed from the base of the PR and between df9b7bb and adfb889.

📒 Files selected for processing (1)

• inc/sso/class-magic-link.php

🚧 Files skipped from review as they are similar to previous changes (1)

• inc/sso/class-magic-link.php

---

📝 Walkthrough

Walkthrough

Eight isolated defensive fixes: a missing break in the BRL currency case, a default initialization for unknown discount types, null-dereference protection in the migration installer, division-by-zero guard in membership normalization, type and null filtering for pending sites, Cloudflare API response validation, hestia DNS content field fallback, and a site-existence check in magic-link generation.

Changes

Defensive Safety Fixes

Layer / File(s)	Summary
Currency and discount utility safety fixes inc/functions/currency.php, inc/functions/discount-code.php	Adds break; after the BRL case to prevent fall-through into BYN; initializes $discounted_price to $base_price so unknown discount types return the original price instead of operating on an undefined variable.
Membership, Site, and Migrator null/type guards inc/models/class-membership.php, inc/models/class-site.php, inc/installers/class-migrator.php	Early-returns the raw amount when duration is zero to avoid division by zero; adds a Site instance check with array_filter/array_values for pending-site deserialization; requires both $membership and $customer before calling $customer->get_id().
Cloudflare, DNS record, and magic-link guards inc/integrations/host-providers/class-cloudflare-host-provider.php, inc/integrations/host-providers/class-dns-record.php, inc/sso/class-magic-link.php	Wraps the Cloudflare zone-ID loop in WP_Error, non-empty, and iterable checks; prefers $data['content'] over $data['value'] in the hestia DNS record mapping; returns false from generate_magic_link() when no site is found and reuses the earlier site lookup for URL construction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A break was missing, the BRL ran away,
A division by zero nearly ruined my day.
Null pointers lurking, the site check was bare—
Now guards stand watch with the greatest of care.
Each safety patch small, but together they shine,
The rabbit hops forward on code that's just fine! 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main focus of the changeset: introducing multiple robustness fixes for null dereferences, division by zero, and control-flow issues across eight files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[superdav42]

Stuck-merge detector: PR has been merge-eligible but unmerged past the threshold

The pulse merge pass has classified PR #1380 as STUCK_OTHER and it has been sitting unmerged longer than AIDEVOPS_MERGE_STUCK_AGE_MINUTES (currently 240m). The deterministic merge gates are evaluated every cycle (~120s) and this PR has consistently failed them.

Failing checks on PR #1380

• (no FAILURE entries in rollup; check rollup manually)

Worker guidance for the next attempt

1. Read PR Robustness: null-deref, div-by-zero and control-flow guards #1380 body + the latest check run logs:

   gh pr checks 1380 --repo Ultimate-Multisite/ultimate-multisite

2. If the failing checks are environment/Setup-step (Format, Lint, Typecheck all FAIL at the same step), the canonical default branch likely has a broken lockfile or a CI infra change — fix at the base, not on this PR. Look for a sibling outage meta-issue in this repo (filed by the same detector) before forking off here.
3. If the failures are PR-specific (e.g. a Typecheck error introduced by this PR's code), rebase onto the latest default branch and address the diagnosed errors. Use full-loop-helper.sh start from the linked PR's worktree.
4. If the linked issue body lacks the worker-ready file paths and verification commands required by t1900, post a comment naming the missing context before dispatching another worker — the next attempt will burn tokens on exploration otherwise.

Why you're seeing this

Every 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 AIDEVOPS_MERGE_STUCK_AGE_MINUTES minutes idle. This comment is posted exactly once per linked issue — repeated stuck cycles will NOT spam the thread. If the PR merges and the issue is reopened later with a fresh stuck PR, the marker will allow a second comment.

_{Posted automatically by pulse-merge-stuck.sh (t3193 / GH#21895). Threshold env: AIDEVOPS_MERGE_STUCK_AGE_MINUTES=240.}

---

aidevops.sh v3.20.57 automated scan.

[coderabbitai]

Actionable comments posted: 1

🤖 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/sso/class-magic-link.php`:
- Around line 128-130: The site existence check for `wu_get_site($site_id)`
occurs after the token has already been persisted via `set_transient()` in the
Magic Link class. If the site lookup fails, this leaves an orphaned transient
until TTL expiry. Move the `wu_get_site($site_id)` call and the null guard check
before the `set_transient()` calls to ensure the site exists before any token is
stored. This prevents unnecessary transient writes when the site validation
fails.

🪄 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: a90e24e5-0493-4856-90b4-064a5c0da1ae

📥 Commits

Reviewing files that changed from the base of the PR and between c56936d and df9b7bb.

📒 Files selected for processing (8)

• inc/functions/currency.php
• inc/functions/discount-code.php
• inc/installers/class-migrator.php
• inc/integrations/host-providers/class-cloudflare-host-provider.php
• inc/integrations/host-providers/class-dns-record.php
• inc/models/class-membership.php
• inc/models/class-site.php
• inc/sso/class-magic-link.php