Merge development hardening and upstream security fixes#1318
Merge development hardening and upstream security fixes#1318zombiesis wants to merge 6 commits intoCtrlpanel-gg:developmentfrom
Conversation
- secure payment gateway callbacks, checkout paths, and webhook fulfillment - close admin authorization gaps across settings, vouchers, coupons, partners, store, and user flows - harden API token scope handling, sensitive-field exposure, notification ownership, and audit logging - sanitize terms and invoice rich text, tighten activity logging, and add security headers - fix race conditions around vouchers and billing, add unique voucher pivot migration, and improve test coverage - refresh built frontend assets and include the security audit notes
- stop re-selecting the latest notification after inserting the fixture - keep the generated notification UUID and assert against the exact record - preserve the green full-suite result under combined test execution
- replace the abandoned reCAPTCHA and PayPal integrations with maintained internal/runtime implementations and updated payment gateway settings - harden Discord OAuth, payment callbacks, referral handling, registration, API user/server sync, and Pterodactyl client parsing/ownership flows - fix server creation and upgrade consistency, overview/dashboard aggregation, invoice numbering, notification batching, validation gaps, and active-client role handling - update tests, frontend build artifacts, and archive both review reports with the second-pass fixes applied
There was a problem hiding this comment.
Pull request overview
This PR merges upstream development into the fork while preserving/expanding local hardening, focusing heavily on RBAC, API token scoping, XSS mitigation, safer installer behavior, and more defensive payment/server lifecycle logic.
Changes:
- Added scoped, revocable application API tokens (abilities, expiry) + API audit trail + per-endpoint scope middleware.
- Reduced XSS risk by sanitizing rich-text settings/content and tightening escaping across Blade views and datatables.
- Hardened operational flows: installer lock behavior, payment callbacks, server billing/suspension, and concurrency-sensitive updates.
Reviewed changes
Copilot reviewed 180 out of 188 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| themes/default/vite.config.js | Ensures Vite build manifest generation aligns with runtime asset loading. |
| themes/default/views/.blade.php + themes/BlueInfinity/ | Escaping/sanitization improvements, safer POST actions (CSRF), and rel=noopener additions. |
| app/Support/HtmlSanitizer.php | Introduces centralized rich-text sanitization used by settings and templates. |
| routes/api.php + app/Http/Middleware/* | Adds API throttling, audit logging, and fine-grained scope enforcement. |
| database/migrations/* | Adds API token table, encrypts existing secret settings, and adds uniqueness constraints for voucher redemption. |
| public/installer/** | Tightens installer enablement rules and improves shell execution / SQL safety. |
| tests/** + phpunit.xml | Updates tests for new auth/RBAC model and adds callback/authorization coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'mail_encryption' => $_POST['encryption'] === 'null' ? null : $_POST['encryption'], | ||
| 'mail_from_address' => $_POST['user'], | ||
| 'mail_from_name' => $_POST['user'], | ||
| ]; | ||
|
|
||
| foreach ($values as $key => $value) { | ||
| wh_log("[MailSettings] Setting" . $key , 'debug'); | ||
| $safeValue = escapeshellarg($value); | ||
|
|
||
| wh_log("[MailSettings] Setting " . $key , 'debug'); | ||
| try { | ||
| run_console("php artisan settings:set 'MailSettings' '$key' $safeValue", null, null, null, false); | ||
| run_console("php artisan settings:set 'MailSettings' " . escapeshellarg($key) . ' ' . escapeshellarg((string) $value), null, null, null, false); | ||
| } catch (\Exception $e) { |
There was a problem hiding this comment.
mail_encryption is normalized to null in $values, but the command building casts (string) $value before escapeshellarg(). When $value is null this becomes an empty string, so the installer will persist "" instead of null, changing semantics for nullable settings.
| $envPath = __DIR__ . '/../.env'; | ||
|
|
||
| if (! file_exists($envPath)) { | ||
| file_put_contents($envPath, ''); | ||
| } | ||
|
|
There was a problem hiding this comment.
This test bootstrap writes an empty .env file into the repository root when none exists. That introduces side effects (dirty working tree, potential permission issues in CI, and surprising local behavior) that tests should avoid. Prefer configuring env via phpunit.xml / $_ENV only, or use .env.testing rather than creating .env at runtime.
| $deduplicatedRows = DB::table('user_voucher') | ||
| ->select( | ||
| 'user_id', | ||
| 'voucher_id', | ||
| DB::raw('MIN(created_at) as created_at'), | ||
| DB::raw('MIN(updated_at) as updated_at') | ||
| ) | ||
| ->groupBy('user_id', 'voucher_id') | ||
| ->get(); | ||
|
|
||
| DB::table('user_voucher')->delete(); | ||
| foreach ($deduplicatedRows as $row) { | ||
| DB::table('user_voucher')->insert((array) $row); | ||
| } | ||
|
|
||
| Schema::table('user_voucher', function (Blueprint $table) { | ||
| $table->unique(['user_id', 'voucher_id']); | ||
| }); |
There was a problem hiding this comment.
This migration deletes all rows from user_voucher and reinserts the deduplicated set without wrapping the operation in a transaction. If the process is interrupted (or an insert fails) the table can be left partially empty/corrupted before the unique index is added. Consider wrapping the dedupe+delete+insert+index creation in a DB transaction (or using a safer in-place dedupe strategy) to make the migration atomic.
| @foreach (($useful_links ?? collect()) as $link) | ||
| <li class="nav-item d-none d-sm-inline-block"> | ||
| <a href="{{ $link->link }}" class="nav-link" target="__blank"><i | ||
| class="{{ $link->icon }}"></i> {{ $link->title }}</a> |
There was a problem hiding this comment.
The useful links open in a new tab using target="__blank", which is not a valid target value (should be _blank). Also, links that open a new tab should include rel="noopener noreferrer" to prevent tabnabbing.
simbabimba-dev
force-pushed
the
development
branch
2 times, most recently
from
8650200 to
32334b0
Compare
Summary
This PR brings the fork's
developmentbranch up to date withCtrlpanel-gg/panel:developmentand preserves the local hardening work on top.It includes:
Ctrlpanel-gg/panel:developmentMain changes
Merge notes
The main overlap was in:
Conflict resolution kept the local validation and service-layer hardening while also keeping the latest upstream RBAC/XSS/security protections.
Verification
php artisan test->92 passeddevelopmentwas merged locally before publishing this branch