* Fix WHM activation failing when package already exists (#4560)
- Request listpkgs with want=all and parse both legacy package and modern data.pkg shapes
- On addpkg already-exists, sync via editpkg and continue; only block activation on other errors
- Include WHM reason in Server_Exception and fix placeholder keys
* Add WHM API terms to spellcheck dictionary
* Address review: use typed exception for WHM already-exists, keep reasons out of API errors
- Add Server_PackageExistsException; request() now throws generic messages and translates already-exists at the WHM boundary
- checkPackageExists() catches the typed exception instead of matching getMessage()
* Fix Rector findings: remove unused catch variable and setAccessible call
* Handle modern WHM metadata.result failures in Whm::request
---------
Co-authored-by: opencode <opencode@localhost>
* Sanitize support ticket note content
Note content arrived through the note_create endpoint and was stored
as provided, then rendered in the admin ticket list dialog with
innerHTML. Sanitize new notes as plain text on the way in (matching
how canned response titles are stored), and render the dialog entries
with a text node instead of innerHTML so previously stored content
cannot inject markup either. Preserve line breaks while rendering the
dialog entries.
* Preview massmailer messages in a sandboxed frame
The message preview injected the rendered message body straight into
the admin page with innerHTML, so a message written by one staff
member would run script in the browser of another staff member
previewing it. Render the body in a sandboxed iframe instead, using
the same posture as the email template preview, and assign srcdoc as
a property so the body is never parsed in the page context.
* Reject support ticket notes that are empty after sanitization
A note whose content is entirely markup survived RequiredParams
(which only checks that the parameter exists) and, now that note
content is sanitized to plain text on storage, would be persisted as
an empty row. Return a validation error instead.
* Harden guest session-state API calls against cross-site requests
Extend the guest authentication guard to the session-bound cart mutations
(cart add/remove item, promo apply/remove, reset, currency change),
requiring the pre-login session nonce and a same-origin context like
client login, signup, and staff login already do. Cart changes accept GET
with the nonce in the query string, matching _checkCSRFToken()'s token
sources, so shipped fb_api_link usage keeps working.
Also pin an explicit SameSite=Lax on the session cookie when
security.mode is not strict: an unset attribute leaves cookie behavior to
each browser's default, and some browsers send cookies on cross-site
requests when it is missing. The installer session already uses Lax.
* Default queued cookies to SameSite Lax
CookieQueue now defaults to Lax so locale and timezone preference
cookies carry an explicit policy instead of leaving the behavior to
each browser's unset-attribute default. The CSRF cookie keeps its
explicit Strict, and callers can still pass null to omit the
attribute.
* Sign guest E2E requests with the pre-login nonce
The Live Tests failures on the cart guard came from the test helper
keeping its own copy of the guarded-endpoint list (login and signup
only), so guest cart calls went out unsigned and were rejected with
403. Attach the nonce to every guest request instead - the dispatcher
strips the parameter where it is not used - and let guest GET requests
carry it in the query string, which _checkCSRFToken() already accepts.
* Set the cURL URL after appending guest GET parameters
CURLOPT_URL copies the value at set time, so mutating $url after
curl_setopt_array() left the handle pointing at the query-less URL and
guest GET requests went out without the nonce or parameters. Re-set
CURLOPT_URL once the query string is appended.
* Update dependency ckeditor5 to v49
* Add editor crash recovery and complete the CKEditor 5 v49 migration
CKEditor 5 v49 removed the Watchdog, so crashed editors are no longer
restarted automatically. The editor wrapper now recreates a crashed
editor from the content synced to its form element (up to 3 times
within a 5s window) and locks it read-only if it keeps crashing, so
the last known-good content is never lost and forms stay submittable.
Also replaces the deprecated ClassicEditor.create() overload with the
config-only form, and drops the dark-mode inline-style hack from the
wysiwyg() Twig function - the refreshed v49 theme pins the editor
colors itself.
* [CodeFactor] Apply fixes to commit b6aa70c
* Address review: reuse editor wrapper and clean up bookkeeping
Recovery now swaps the recreated CKEditor instance into the existing
normalized wrapper via replaceRaw() instead of building a second
wrapper, so references held by callers (element.editor, the resolved
create() promise) stay valid and their change listeners survive.
Editor destruction now unregisters the editor from the page-level
error registry and drops its crash record, instead of retaining both
for the life of the page.
---------
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Adam Daley <admdly@users.noreply.github.com>
Co-authored-by: codefactor-io <support@codefactor.io>
Extend the guest authentication guard to cover staff_login with the same
POST-only, pre-login session nonce, and origin checks that client login
and signup already enforce, and ship the CSRF token in the admin login
form. Adds unit coverage for the staff path.
Also remove the post-login redirect branch in _apiCall(): it compared
$method against 'login', but $method holds the full call name there
(staff_login/client_login), so the branch was never reachable. All
shipped login forms submit via the API bundle and redirect client-side.
* Address review comments on #4541: document UGX exclusion, hardcode Stripe decimal lists
- Rename STRIPE_ZERO_DECIMAL_CURRENCIES to STRIPE_ZERO_DECIMAL_CHARGE_CURRENCIES
and document why UGX is excluded (zero-decimal per Stripe's list, but its
special-case row requires charge amounts x100), plus ISK/HUF/TWD notes.
- Replace the CLDR-based three-decimal check with a hardcoded
STRIPE_THREE_DECIMAL_CURRENCIES list (BHD, JOD, KWD, OMR, TND); drop the
Symfony Intl dependency from this helper.
- Reword the StripeTest comment to state the UGX exception explicitly.
* Extend Stripe currency test coverage in #4541 follow-up
- getAmountInMinorUnits: add BHD (second three-decimal code), AFN/YER
(more CLDR-zero codes Stripe charges x100), TWD (payout-zero/charge-two
special case), unknown-code fallback (XXX) and lowercase inputs.
- Add getAmountFromMinorUnits round-trip test via invokePrivateMethod so the
read-back path is covered too.
* 🐛 Send Stripe amounts in Stripe's units, not CLDR's
The adapter scaled amounts by CLDR's fraction digits, which are 0 for
COP, HUF, IDR, ISK, PKR, UGX and several other currencies that Stripe
expects with two decimals, so it sent 1/100 of the invoice total and
read Stripe's amounts back with the same factor. Use Stripe's rules:
its zero-decimal currencies as-is, three-decimal ones with three
digits, everything else (ISK and UGX included) x100.
* ✏️ Avoid an unknown word in the Stripe currency test comment
* 🐛 Keep the adapter display size for custom payment gateway logos
Custom gateway images rendered in a fixed 150x50 box, letterboxing
portrait logos. Reuse the adapter logo dimensions instead.
Follow-up to #4500.
* Use readonly anonymous class in gateway logo size test
* Harden Stripe renewal paid-skip for inconclusive billing reasons
Only skip the already-paid original invoice for the initial payment or
when billing_reason is missing (ambiguous: duplicate initial webhook vs
renewal — skipping avoids double-crediting). Adds regression tests that
a duplicate initial webhook is still skipped and documents the double
DB fetch as an identity-map hit.
* Rename test identifiers to satisfy spellcheck
🐛 Record Stripe renewal payments after the first invoice is paid
The subscription metadata always points at the first invoice of the
subscription, so the "already paid" skip in invoice.paid handling
returned early for every renewal: no funds were added and the renewal
invoice stayed unpaid. Apply the skip to the initial payment only.
Co-authored-by: Adam Daley <admdly@users.noreply.github.com>
* Fix cron-sent emails rendering dates in en_US instead of configured locale
In CLI/cron context there is no Accept-Language header, so
Locale::acceptFromHttp() fails and Locale::lookup() with an empty
detected locale returns the first enabled locale (en_US) instead of
null, overriding the configured i18n.locale default. Guard
getBrowserLocale() against empty/unparsable headers so it falls back
to the configured default, and stop queueing empty locale cookies.
Also plumb an explicit per-recipient locale through email rendering:
TwigFactory::createEmailEnvironment(), System render helpers, and
Email::sendTemplate() now honor the recipient's language
(client lang, per staff member on broadcasts), outranking browser
auto-detection, with Massmailer rendering per-client too.
Fixes#4524
* Fix tests: simulate header-less requests, update subject render expectation
Symfony's Request::create() seeds a default Accept-Language header,
so strip it to reproduce the CLI/cron condition in the new
getActiveLocale fallback tests. Update the existing
renderEmailSubjectString test for the new locale parameter.
🐛 Soften PayPal item_number echo check to a warning
PayPal freezes item_number at subscription signup while the invoice
number can change (renumbering, issuing), so the strict check added in
#4424 failed legitimate renewal payments for older subscriptions.
Downgrade the item_number mismatch to a warning log and keep
processing; the stable internal invoice id reference remains a hard
error. The signed callback binding, subscription linkage, and amount
checks remain the payment binding.
Fixes#4533
* Refresh default branding assets
Add updated light and dark logos plus an SVG favicon, and point installer and default company branding settings to the new assets. Update shared theme tests to use the renamed light logo.
* Complete default branding asset migration
* Preserve branding assets still referenced by settings
* ✨ Add configurable send time for invoice reminders
Adds an invoice_reminder_send_time setting (HH:MM, empty disables) so
automatic before/after-due reminders go out on the first cron run at or
after the configured time instead of after midnight. Both handlers defer
before the atomic reminded_at claim, leaving manual sends immediate.
Closes#4478
* Fix spelling: unparseable -> empty or invalid
* Require two-digit hours for invoice reminder send time
The settings form uses a time input, which only round-trips canonical
HH:MM values. Resolves CodeRabbit review thread.
* Add regression tests for null invoice PDF party data
Covers getBuyerData()/getSellerData() with null, whitespace-only, and
missing buyer/seller fields, promoting PHP deprecations/warnings to
failures. Guards against a recurrence of Sentry FOSSBILLING-QT8
(trim(null) deprecation on the invoice PDF path, reported on #4423).
* Assert exact invoice PDF party-data keys in regression test
Addresses CodeRabbit review: not->toHaveKeys() passes when any single
listed key is absent, so compare array_keys() against the expected set
to reject any incorrectly retained blank field.
* Cover whitespace-only seller field in PDF party-data test
Uses a whitespace-only address_1 in the seller fixture so the
empty-result assertion also verifies getSellerData() filters
whitespace-only values, mirroring the buyer fixture.
* 🐛 Treat missing extension manifests as user errors
Activating a module whose files are incomplete threw a generic
Exception, which the API layer reported to Sentry (FOSSBILLING-QTS).
Missing/invalid manifests are now InformationExceptions so they
return a friendly API error instead, activation validates the
manifest before persisting anything, and handled API errors no
longer capture InformationExceptions to Sentry.
* Fix Extension ServiceTest mocks for hasManifest pre-check
The activate() manifest guard added a hasManifest() call the existing
Module mocks didn't expect. Stub it true in the happy-path tests.
* Rename test fixture to a dictionary word (missingmod -> nonexistent)
* Validate full manifest before persisting on activation
hasManifest() only proves the file exists; invalid JSON still reached
persist and the Before event before activate() rejected it. Read the
manifest up front so both cases fail before any side effects.
* Reject non-object JSON manifests as invalid
Valid scalar JSON (null, string, number) decoded fine but crashed
array_merge with a TypeError. Treat it as an invalid manifest.
* Reject JSON list manifests as invalid
A top-level list decodes to a PHP array and sailed through array_merge
as default metadata. Manifests must be JSON objects.
* Validate manifest is a JSON object before decoding
Decode once as an object so top-level lists and scalars are rejected
while an empty object stays valid, then keep the associative decode
for the merge flow. All manifest failures now share code 5897.
Manual captureException in the API controller reported every handled
exception, including user-facing validation errors such as sandboxed
email template rejections triggered by scanner probes (FOSSBILLING-R4S).
Only unexpected failures now reach Sentry, matching sanitizeGuestError.
Adam Daley