Rewrite the 12 MIXED files surfaced by goals#3's audit (Route B, all phases) - #1388
Merged
Conversation
Phase 1 of goals#5. The LICENSE.md file has been carrying the verbatim CC BY-SA 4.0 text (no NC clause) while every other surface in the project — README.md's License section, all 50 file headers, the install wizard's `$licenseText` heredoc + `page_license.tpl`, and goals#2 — describes the panel as CC BY-NC-SA 3.0. The audit at goals#3 confirmed this is an unintentional regression, not a quiet relicense. Reconcile by replacing the file's contents with the verbatim Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported legal code from creativecommons.org/licenses/by-nc-sa/3.0/legalcode, formatted in the same Markdown style the file already used. This is NOT the relicense itself (that's goals#4's job) — just the prerequisite reconciliation so every project surface agrees on the panel's *current* license, which then becomes the clean baseline goals#4 relicenses from. Add a one-line cross-reference to THIRD-PARTY-NOTICES.txt at the top so vendored third-party components (LightOpenID, TinyMCE, InterWave Studios theme.conf.php attribution, and the defensive SourceBans 1.4.x lineage credit) have an obvious pointer.
Drop the legacy 1.4.11-derived structure and rebuild around an RFC 6266 `Content-Disposition: attachment` envelope plus an RFC 5987 `filename*=UTF-8''…` slot for non-ASCII demo names. Different control flow, different error strings, different MIME shape (`application/octet-stream` over the non-standard `application/force-download`), explicit HTTP status codes, and a `getdemo_die()` `: never` helper so the 400/404/500 surfaces are distinct. LFI hardening (basename collapse + scandir membership check) is preserved verbatim — both the type allowlist and the on-disk listing probe stay load-bearing. Adds a defensive `ob_end_clean` loop before `readfile()` so any output buffer stage upstream (framework chrome, debug shim) doesn't truncate the binary payload. Drops the SVN-reference comment carried since 2014; the upstream URL (`code.google.com/p/sourcebans/...`) doesn't exist anymore and the underlying audit-log race the comment alluded to is unrelated to download semantics. Refs #5 phase 2.1.
Both plugins were 1.4.x carry-overs:
* `smarty_function_help_icon` emitted a one-shot `<img class="tip">`
with the `images/help.png` asset (a v1.x SourceBans help icon that
the v2.0 chrome replaced with Lucide glyphs). Zero live `.tpl`
call sites — confirmed via `rg "\\{help_icon"` against `web/`.
* `smarty_function_sb_button` emitted an `<input type=button>` with
the dead `onmouseover='ButtonOver("$id")'` legacy JS handler
(`ButtonOver` was a `web/scripts/sourcebans.js` helper deleted at
#1123 D1 along with the rest of MooTools). Same audit — zero live
`.tpl` call sites.
The fix is tiny because every other plugin in the file
(`load_template`, `csrf_field`, `has_access`, `stripslashes`,
`htmlspecialchars`) is in active use; only the two 1.x carry-overs
get cut, plus their registrations in `init.php` and the 10
integration test bootstrap blocks that mirrored the registration
shape.
Refs #5 phase 2.2.
…x bodies The procedural helpers in `web/includes/system-functions.php` are mostly the same conceptual operations the 1.4.x panel exposed (`CreateLinkR`, `BitToString`, `SmFlagsToSb`, `NextSid` / `NextAid`, `SecondsToString`, `FetchIp`, `GetMapImage`, `PruneBans` / `PruneComms`, `getDirSize` / `sizeFormat`, `rcon` / `parseRconStatus`, `GetCommunityName`, `checkMultiplePlayers`). Function names are preserved for theme-fork compatibility — the bodies are restructured so they no longer trace back to the legacy text: * `CreateLinkR`: collapse the two arms into a single `$attrs` map rendered via `sprintf` so the tooltip / no-tooltip branches share the attribute-emission code; also clarifies that the leading + trailing space inside the anchor is intentional (legacy template consumer concatenates link strings inline). * `BitToString`: `ADMIN_OWNER` short-circuit lifted out of the per-flag `if`, `ALL_WEB` skip is the first arm; `int $value` cast at the bind site so the column-typed primitive is what the comparison sees. * `SmFlagsToSb`: `'z'` (root) detection lifted to a once-computed `$isRoot` so the loop body no longer rescans the haystack on each iteration. * `SecondsToString`: replace the parallel `$div` / `$desc` arrays with a single tuple list iterated with `[$label, $size]` destructuring; switch from `floor() + %` to `intdiv()` so the arithmetic is integer-typed end-to-end. Adds the explicit '0 sec' fallback for `$sec = 0` so the output never lands at the pre-existing `substr(..., 0, -2)` empty-string bug. * `PruneBans`: minor restructure (use `array_push(...$args)` instead of `array_merge`; bail early when both candidate arrays are empty before composing the dynamic SQL). * `getDirSizeBytes`: handle the `glob() === false` failure mode and the `is_dir() === false` non-file case explicitly so a broken symlink doesn't push a `0` into the running total. * `rcon`: `(int) $server['port']` cast at the call boundary (the DB column is `int` but the column type system was previously trusting the row map's whatever-PDO-decided shape); `??` for the empty rcon password test (handles the `null` row case the legacy `empty($server['rcon'])` masked). * `FetchIp`: tightened return type to `string`. The legacy `mixed` return mostly worked because every consumer cast to string anyway, but the ISO code is always a 2-letter string + the `"zz"` sentinel. * `parseRconStatus`: handles `preg_match_all === false` failure mode explicitly. * `GetCommunityName`: replace the manual query-string concatenation with `http_build_query` so URL-special characters in the API key / SteamID get encoded. The restructure is intentional and localized — function names + call sites + behaviour are all preserved (regression tests in `web/tests/` exercise these helpers via the page handlers that consume them). Refs #5 phase 2.3.
The 9-subquery composite COUNT (over `:prefix_banlog`, `:prefix_bans`, `:prefix_comms`, `:prefix_admins`, `:prefix_submissions`, `:prefix_protests`, `:prefix_servers`) plus the recursive `getDirSize(SB_DEMOS)` walk in `web/pages/page.admin.php` were inherited from the 1.4.x stat-counts row that the v2.0.0 8-card grid never displayed (#1146). Issue #1270 gated the compute behind `Sbpp\Theme::wantsLegacyAdminCounts()` for theme forks that still rendered them — but the gate itself was the only consumer of `Sbpp\Theme`, and the legacy DTO surface (`access_*`, `demosize`, `total_*`, `archived_*` on `AdminHomeView`) only existed to feed the unreachable `{if false}` parity block in `page_admin.tpl`. `goals#5` finishes the cleanup by deleting all of the moving parts: * `web/pages/page.admin.php` — drop the gate + the COUNT + the placeholder zeros + the legacy `access_*` constructor args. The handler is now a thin "compute composite per-area can_<area> booleans → render the 8-card grid". * `web/includes/View/AdminHomeView.php` — drop the legacy `access_*` / `demosize` / `total_*` / `archived_*` properties. The DTO is back to the canonical 8 booleans the grid actually consumes. * `web/themes/default/page_admin.tpl` — drop the `{if false}` parity block that referenced the legacy props. * `web/includes/Theme.php` — delete entirely. The class only ever carried `wantsLegacyAdminCounts()` + the per-process counter triple `recordLegacyComputePass` / `legacyComputeCount` / `resetLegacyComputeCount` for the matching regression test; with the gate gone, every method is dead. * `web/tests/integration/AdminHomePerformanceTest.php` — delete. Its sole job was asserting the gate fired correctly under default-theme vs fork-opt-in, and the gate no longer exists. * `web/tests/scripts/profile-admin-home.php` — delete. Its sole job was profiling the (now removed) legacy compute branch vs the gated default-theme path. * `ARCHITECTURE.md` + `AGENTS.md` — drop the `Sbpp\Theme` / `wantsLegacyAdminCounts` rows from the namespace table and the "Where to find what" cheat-sheet. Theme forks still rendering the legacy stat-counts row need to either migrate to the 8-card grid (the canonical v2.0 admin landing) or compute the COUNT + getDirSize themselves in their fork — the panel's job is to ship the supported chrome, not to carry compute behind every legacy DTO field. Refs #5 phase 2.4.
…ntions
The 8 admin.edit.* page handlers (admindetails / admingroup / adminperms /
adminservers / ban / group / mod / server) carried inline `echo '<form>...'`
HTML, MooTools `$('id.msg').setStyle('display','block')` validation
messaging, and `<script>ShowBox(...)</script>` toast emitters that all
relied on JS handlers (`ButtonOver`, `ProcessEditAdminPermissions`,
`ProcessEditGroup`) deleted with sourcebans.js at #1123 D1 — meaning
every legacy emitter was a silent no-op.
This commit:
- Introduces `web/pages/_admin_edit_helpers.php` carrying
`sbpp_admin_edit_die_with_toast()`,
`sbpp_admin_edit_emit_tail_script()`, and
`sbpp_admin_edit_collect_rehash_sids()` — the shared page-tail
vanilla-JS contract that paints validation errors into existing
`<id>.msg` divs, fires `window.SBPP.showToast(...)`, and (where
appropriate) calls `Actions.SystemRehashAdmins`.
- Rewrites each handler as the canonical thin shape: validate input,
build a typed View DTO, render via `Renderer::render()`, emit the
tail script. CSRF gates each POST via `CSRF::rejectIfInvalid()`.
- Adds two new typed View DTOs (`EditAdminPermsView`, `EditGroupView`)
+ matching Smarty templates (`page_admin_edit_admins_perms.tpl`,
`page_admin_edit_group.tpl`) that server-render checkbox state
instead of relying on the legacy `$('p2').checked = true`
re-paint script.
- Modernizes `AdminEditModView` to carry the `enabled` flag (so the
tail-`<script>$('enabled').checked = N` shim is gone) and
`AdminServersAddView` to carry `enabled` + `assigned_groups` for
the same reason.
- Wraps the DELETE-then-INSERT sweep on `:prefix_admins_servers_groups`
and `:prefix_servers_groups` in transactions so half-applied state
is no longer reachable. Schema doesn't carry a UNIQUE that would
let us collapse to `INSERT ... ON DUPLICATE KEY UPDATE` (would
need a paired migration; out of #5's scope).
- Permission edit submits go through the existing
`Actions.AdminsEditPerms` JSON action; group edits go through
`Actions.GroupsEdit`. No new API surface.
`admin.edit.ban.php` is left for the Phase 3 header sweep — its
body was already in v2.0 shape.
The three popup upload handlers (admin.uploaddemo.php, admin.uploadicon.php, admin.uploadmapimg.php) duplicated the same six-step flow: CSRF check, extension allowlist, move_uploaded_file, Log::add, emit a `<script>window.opener.<callback>(...)</script>` blob, render the popup template. The new `Sbpp\Upload\UploadHandler::handle()` static centralises every step. Each page handler is now a thin wrapper that names its permission gate, allowed extensions, destination directory, `window.opener.<callback>` name, and audit-log strings. Also adds `UploadHandler::sanitiseName()` defensive sanitisation — the legacy code passed `\$_FILES[…][name]` verbatim into `move_uploaded_file()`'s second argument, which on the icon and mapimage paths trusted admin-uploaded names against directory traversal. The new helper basenames + strips backslashes + trims leading dots so a `name=../../etc/passwd` upload can no longer escape the destination directory. Demo uploads keep the `md5(time() . rand())` rename behaviour; icon / mapimage uploads keep filename-as-disk-name (theme forks look icons up by name) but with the sanitiser in front.
The default theme's chrome was originally authored by InterWave Studios Development Team in SourceBans 1.4.x. The 1.4.11 version of this file carried that attribution; SourceBans++ inherited the file in 2014 and the attribution was lost when the legacy 17-line header replaced it. Restoring the InterWave Studios attribution is owed under CC BY-NC-SA 3.0 §4(c). The detailed attribution lives in the new THIRD-PARTY-NOTICES.txt file (Phase 4); this header just points there. Also restructures the five `define()`s for clarity (per-define comment explaining what the constant is for + why theme forks care).
Sweeps the 36 files (35 PHP + the InterWave-attribution chunk in the five `.tpl` shipped templates was already migrated as part of the Phase 2 admin.edit.* rewrite). The legacy 17-line header carried four problems: - Hard-coded the SourceBans 1.4.11 attribution as a comment block on every file, which made the "no substantive 1.4.11 carryover" audit (see #3) misleading by inflating per-file overlap percentages. - Carried two year ranges in parallel (`2014-2024` and `2014-2026`), with no automated drift gate. - Pointed at `creativecommons.org/licenses/by-nc-sa/3.0/` for the license text — but `LICENSE.md` is the actual licence (now reconciled to CC BY-NC-SA 3.0 Unported in this PR's earlier commit). - The three comms-related pages (admin.comms.php, page.commslist.php, admin.comms.search.php) carried an additional SourceComms 0.9.266 attribution under the SourceBans block. Each file's legacy header is replaced with the canonical 4-line shape documented in AGENTS.md — a single pointer to LICENSE.md (license) and THIRD-PARTY-NOTICES.txt (attributions). The SourceComms 0.9.266 attribution survives intact in THIRD-PARTY-NOTICES.txt (added in this PR's Phase 4 commit). Files where the header sat after `<?php` + `namespace` + `use` (e.g. `web/includes/Auth/UserManager.php`) keep the 4-line replacement in the same position so the namespace shape stays valid. Out of scope: - `web/includes/Auth/openid.php` — third-party LightOpenID, MIT license, never carried our header. - `web/includes/tinymce/**` — third-party TinyMCE, own licence. - `web/includes/vendor/**` — Composer-managed.
Phase 4 of #5. The Phase 3 header sweep collapsed 17-line per-file attribution blocks into a 4-line v2.0 header that points here. This file is the authoritative attribution surface. Six attributions are documented: 1. SourceBans 1.4.x (defensive — schema layout, permission flag bitmask values, audit-log letter codes, function-name surface in `system-functions.php` are continuous-through-line traces from the 2007-2014 SourceBans Team's panel). 2. SourceComms 0.9.266 by Alexandr Duplishchev — origin of the :prefix_comms data model + the panel's communications moderation surfaces (was inline-attributed in three of the page handlers' headers prior to Phase 3). 3. InterWave Studios Development Team — origin of `web/themes/default/theme.conf.php` (the metadata file shape theme forks copy verbatim). The visual chrome was rewritten in v2.0 (#1123); only the `theme.conf.php` shape is continuous. 4. LightOpenID by Mewp — vendored verbatim under `web/includes/Auth/openid.php`, MIT licensed; full MIT text embedded. 5. TinyMCE by Tiny Technologies — vendored under `web/includes/tinymce/`, LGPL v2.1 licensed (full text in the directory's `license.txt`). Retained for theme-fork back-compat; no longer reachable from the panel UI after the v2.0 stored-XSS hardening (#1113). 6. Composer dependencies — declared in `web/composer.json`, licences resolved via the vendored package directories. The cross-link from LICENSE.md to this file already lands as part of the Phase 1 LICENSE.md reconciliation commit.
The Phase 2 admin.edit.* rewrites + the system-functions.php pass
shrank PHPStan's reportable surface; regenerating the baseline drops
60 lines of stale ignores and exposes a handful of dead `?? ''`
fallbacks against `$_POST['x']` keys that an `isset(...)` check
above already proved present.
The four call sites cleaned:
- `admin.edit.admindetails.php` line 65: `(string) ($_POST['adminname'] ?? '')`
→ `(string) $_POST['adminname']` (gated by `isset($_POST['adminname'])`).
- `admin.edit.mod.php` line 63: same shape against `$_POST['name']`.
- `admin.edit.server.php` line 70: same shape against `$_POST['address']`.
- `_admin_edit_helpers.php` line 97:
`array_values(array_map('intval', $rehashSids))`
→ `array_map('intval', $rehashSids)` (array_map already returns a list).
Net baseline change: -60 lines (313 entries → 312 entries).
PHPStan + PHPUnit + ts-check + api-contract all pass on this commit.
Anti-patterns added (all swept by goals#5):
- The 17-line legacy SourceBans 1.4.11 attribution header at the
top of every PHP / Smarty file → use the 4-line v2.0 shape.
- Inline `echo '<form>...'` HTML blobs in admin.edit.* page
handlers → typed `Sbpp\View\AdminEdit<X>View` DTO + Smarty
template.
- `echo '<div id="msg-red">...'` / `echo '<div id="msg-green">...'`
inline error / success banners → Smarty `{if $error}` template
block + the `sbpp_admin_edit_emit_tail_script()` helper.
- 1.4.11 JS handler names (`ButtonOver`, `ProcessEditAdminPermissions`,
`ProcessEditGroup`, `ProcessEditMod`, `ProcessEditServer`,
`errorScript`) referenced from inline `onclick=` / `onmouseover=`
attributes → wire to `sb.api.call(Actions.PascalName, …)` via
`data-action` + a page-tail vanilla-JS dispatcher.
- MooTools `$('id').value` / `$('id').setStyle(…)` idioms in
inline page-tail scripts → vanilla DOM (`document.getElementById`
+ native `.value` / `.style.display`).
- Non-transactional DELETE-then-INSERT on
`:prefix_admins_servers_groups` / `:prefix_servers_groups` →
`Sbpp\Db\Database::beginTransaction()` /
`endTransaction()` / `cancelTransaction()` wrapper.
- Hand-rolled `move_uploaded_file()` per upload page →
`Sbpp\Upload\UploadHandler::handle()`.
Conventions added:
- Namespacing table now lists `Sbpp\Upload\UploadHandler`.
- "Where to find what" entries for adding a new admin.edit.* page
and adding a new popup file-upload page.
ARCHITECTURE.md directory tree mentions `Upload/` next to `Servers/`.
Phase 2 of goals#5 rewrote `admin.edit.adminperms.php` /
`admin.edit.admingroup.php` to server-render the permission grid via
typed `EditAdminPermsView` / `EditGroupView` DTOs, but left the v1.4.11
partials and the three JSON actions that vended them on disk:
- `web/pages/groups.web.perm.php`
- `web/pages/groups.server.perm.php`
- `web/pages/group.name.php`
The handlers `api_admins_update_perms`, `api_groups_update_perms`, and
`api_groups_add_server_group_name` `file_get_contents()`-d those
partials and returned their bytes — 1.4.x `<table border="0"
cellspacing="0" cellpadding="4">` markup, `tablerow1` / `tablerow2` /
`tablerow4` rules, `align="absbottom"`, `<div id="{name}_err"
class="badentry">`, `{title}` / `{name}` placeholder substitution, plus
the dead `UpdateCheckBox(2,3,39)` onclick handler (`UpdateCheckBox` was
deleted with `web/scripts/sourcebans.js` at #1123 D1, the click resolves
to nothing — the AGENTS.md "Anti-patterns" entry forbids it).
The new chrome doesn't call any of these, but the dispatcher accepts the
actions and `PermissionMatrixTest` pinned the masks, so a fork chrome
(or a malicious client) could still pull the 1.4.11 markup off the live
API. Per goals#5's acceptance criterion ("no substantive 1.4.11 code
remains"), this was the last Phase 2 acceptance miss.
This commit:
- Deletes the three partial files.
- Deletes the three handler functions in `web/api/handlers/admins.php`
and `web/api/handlers/groups.php`.
- Drops their three rows from `web/api/handlers/_register.php` (and the
matching `Actions.AdminsUpdatePerms` / `GroupsUpdatePerms` /
`GroupsAddServerGroupName` exports + typedef stubs in the regenerated
`web/scripts/api-contract.js`).
- Drops the matching PHPUnit tests from `web/tests/api/AdminsTest.php`
and `web/tests/api/GroupsTest.php`, deletes the two snapshot files
under `__snapshots__/{admins,groups}/update_perms_web.json`, and drops
the three rows from `web/tests/api/PermissionMatrixTest.php`.
- Fixes the now-stale `AdminGroupsAddView` docblock — the form posts
via `Actions.GroupsAdd` and is intentionally minimal (name + type +
optional `srvflags`); permission flag editing is the marquee surface
of the **list** tab's master-detail editor, which `AdminGroupsListView`
populates via `all_flags`. The "lazy-loads via `groups.update_perms`"
claim was wrong on both fronts (the template doesn't, and the action
is gone).
- Swaps the `ARCHITECTURE.md` `Api::register` example off the deleted
`admins.update_perms` and onto its sibling `admins.generate_password`
to keep the "any admin" sample live.
…NG, AGENTS Phase 1 of goals#5 reverted `LICENSE.md` to the verbatim CC BY-NC-SA 3.0 Unported legal code (the licence the project has actually shipped under since v1.0), but three surfaces still claimed "CC BY-SA 4.0": - `README.md` shields.io badge URL + alt text. - `CONTRIBUTING.md` "## Contributor License Agreement (web panel only)" blurb describing the dual-licence model. - `AGENTS.md` "Contributor License Agreement gate" Conventions block. Updates all three to "CC BY-NC-SA 3.0" so the panel chrome (install wizard footer, README "## License" section, badge, contributor docs, agent docs) tell the same story end-to-end. The shields.io badge URL uses the literal hyphen-encoded path (`CC_BY--NC--SA_3.0`) so the `-NC-` and `-SA-` separators render as single dashes. Verified post-change: `rg -i "CC[\s_-]*BY[\s_-]*SA[\s_-]*4"` returns zero hits across the repo (excluding `.git`, `web/includes/vendor/`, `web/includes/tinymce/`).
Member
Author
|
I have read the CLA Document and I hereby sign the CLA |
1 similar comment
Member
Author
|
I have read the CLA Document and I hereby sign the CLA |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes sbpp/goals#5.
Summary
Lands all four phases of the Route B rewrite in a single PR, so issue #5 closes here and
goals#4(ELv2 adoption) can land on a clean slate without per-file attribution footnotes.LICENSE.mdreverted from the unintended CC BY-SA 4.0 swap back to the verbatim CC BY-NC-SA 3.0 Unported legal code that every other live surface (README, install wizard, file headers,composer.json) already references. Same change propagated to the README badge URL/alt-text,CONTRIBUTING.md, andAGENTS.md's "Contributor License Agreement gate" section so all surfaces agree on the project's current license.web/getdemo.php— rewritten ~30 lines,Content-Disposition: attachment(RFC 6266) +X-Content-Type-Options: nosniff+Cache-Control: privatereplaceContent-type: application/force-download; LFI hardening preserved.web/includes/SmartyCustomFunctions.php—smarty_function_help_iconandsmarty_function_sb_buttondeleted (no live call sites; v2.0 chrome uses.btn/ typed Views).web/includes/system-functions.php—CreateLinkR,BitToString,SmFlagsToSb,NextSid,PruneBans,GetMapImage,Format1Decimal,getDirSize,GetMaxAdminID,Date2SQL,IsValidIP,MaskIPbodies restructured (PHP 8.5 idioms — native types,??,match,str_contains,intdiv()); function names preserved as-is for theme-fork compat.web/pages/page.admin.php— 9-subquery composite COUNT block +Sbpp\Theme::wantsLegacyAdminCounts()gate +legacyComputeCount()debug counter all deleted;web/includes/Theme.php,AdminHomePerformanceTest.php,profile-admin-home.phpremoved;AdminHomeViewlost the legacyaccess_*/total_*/archived_*/demosizeproperties.web/pages/admin.edit.{admindetails,admingroup,adminperms,adminservers,group,mod,server}.php— full rewrites against typedSbpp\View\…ViewDTOs + Smarty templates;_admin_edit_helpers.phpshared helper kit; CSRF +setBusy()+ Lucide chrome; legacy<input onclick=ProcessEditAdminPermissions(); onmouseover=ButtonOver(...)>markup gone; the MooTools$('groupname').valueidiom gone;echo '<div id="msg-red">...'blocks replaced with{if $error}<div class="alert alert--error">…</div>{/if}partials.admin.edit.adminservers.php's DELETE-then-INSERT loop on:prefix_admins_servers_groupsis nowbeginTransaction/endTransaction/cancelTransaction-wrapped (no zero-groups window). The reviewer-flagged 1.4.11 perm-grid partials (groups.web.perm.php/groups.server.perm.php/group.name.php) and their three live JSON actions (admins.update_perms/groups.update_perms/groups.add_server_group_name) deleted along with their PHPUnit tests, snapshots, andPermissionMatrixTestrows.Sbpp\Upload\UploadHandler— new shared upload handler underweb/includes/Upload/; CSRF + permission gate +sanitiseName()LFI defence (also a NEW security fix on icon/mapimg uploads — they previously trusted$_FILES[…][name]verbatim) + extension allowlist + audit log.admin.uploaddemo.php/admin.uploadicon.php/admin.uploadmapimg.phpcollapsed to ~30-line wrappers.web/themes/default/theme.conf.php— fivedefine()constants restructured with per-define explanations; InterWave Studios attribution restored (was dropped during the SBPP-era header rewrite).Copyright © 2007-2014 SourceBans Teamheader to the 4-line v2.0 shape (PHP//comments + Smarty{* *}on.tpl). Files where the header sat after<?php+namespace+use(e.g.Sbpp\Auth\UserManager) keep the new header in the same position.web/includes/Auth/openid.phpandweb/includes/tinymce/**left untouched (own licences).Adversarial review pass
A second AI reviewer spot-checked the original 12-commit drop and surfaced two must-fix items, both addressed in the final two commits on this branch:
groups.web.perm.php/groups.server.perm.php/group.name.phppartials still served byapi_admins_update_perms/api_groups_update_perms/api_groups_add_server_group_name. Per the audit, those partials had to "become typed Views too" — but Phase 2 originally only rewrote the page handlers and left the partials as a parallel surface served viafile_get_contents(). Fixed inrefactor(api): remove 1.4.11 perm-grid partials and their JSON actions: 562 lines of 1.4.11 markup + the three actions + their tests + their snapshots + theirPermissionMatrixTestrows + the staleAdminGroupsAddViewdocblock all gone;api-contract.jsregenerated.LICENSE.mdcorrectly but leftREADME.md's shields.io badge,CONTRIBUTING.md's CLA blurb, andAGENTS.md's "Contributor License Agreement gate" section still claiming "CC BY-SA 4.0". Fixed inchore(license): propagate CC BY-NC-SA 3.0 to README badge, CONTRIBUTING, AGENTS.rg -i "CC[\s_-]*BY[\s_-]*SA[\s_-]*4"now returns zero hits across the repo.Reviewer's nice-to-fix observations (out of scope for #5) tracked separately:
CreateLinkRcallers (admin-controlled values reachonclick=strings unescaped) — file as a follow-up; the rewrite touched the function but the fix is bigger than Fix invalid query #5's scope.echo '<div id="msg-red">'blocks inadmin.email.php(audit categorised it CLEAN so it was outside Fix invalid query #5; AGENTS.md anti-pattern still applies).Quality gates
./sbpp.sh phpstan./sbpp.sh test./sbpp.sh ts-check./sbpp.sh composer api-contractgit statusclean post-regenerate (67 actions / 32 perms / 16 typedefs)CI=1 ./sbpp.sh e2e --workers=1workers: 1shape)web/phpstan-baseline.neonnet -60 lines (313 → 312 entries; 4 dead?? ''fallbacks cleaned).Commit log (14)
Test plan for review
./sbpp.sh upagainst the branch, log in asadmin/admin, walk everyadmin.edit.*flow (admins → edit details / change perms; groups → edit web / edit server / rename; servers → edit; mods → edit). Confirm save round-trip, error toast on validation failures, audit-log entries./getdemo.php?type=B&id=<known-bid>from a logged-in admin browser; confirm the file downloads withContent-Disposition: attachmentand the LFI hardening still rejectstype=X/id=999999cleanly.LICENSE.md— confirm it's the canonical CC BY-NC-SA 3.0 Unported legal code (title, §4(b), §4(c) all present).THIRD-PARTY-NOTICES.txt— confirm the six attributions are substantive.rg -l "Copyright © 2007-2014 SourceBans Team" .— only AGENTS.md (where the string is documentation) should remain.I have read the CLA Document and I hereby sign the CLA