* Fix core-update login lockout and mid-update file-swap race
performUpdate() destroyed the admin session and deferred database
patches to a later, manually-triggered finalization step. If the
update changed a table the login flow depends on (e.g. adding
admin.system_name), the admin could no longer log back in to reach
that finalization screen at all, since Doctrine's login query already
expected the new column. Apply patches (via
UpdateFinalization::finalizeUpdate()) before destroying the session
instead of after, so the schema is already patched by the time the
admin needs to log back in.
Also close the underlying file-swap race: performUpdate() extracts
the release archive directly on top of the live, still-serving
PATH_ROOT. A concurrent request landing mid-extraction can autoload a
torn mix of old and new class files and fatal with an "incompatible
declaration" error. Write a lock file before extraction/patching and
have load.php refuse to serve any request while it exists, checking
before the Composer autoloader (and therefore any FOSSBilling class)
is even loaded. The lock is removed in a finally block and
self-expires after 10 minutes as a failsafe if the process is killed
outright (host timeout, OOM) before that can run.
Fixes#4159
* Address CodeRabbit review: lock mutex, zip-slip guard, test cleanup
- Update::performUpdate() now acquires the update lock with an atomic
exclusive fopen('x') instead of dumpFile(), so two updates triggered
at the same time can't both extract into PATH_ROOT concurrently; the
loser gets a clear "already in progress" error instead of corrupting
the extraction. A leftover lock older than 10 minutes is still
treated as abandoned so a killed process can't wedge future updates.
- The lock's mtime is refreshed after extraction (before patches run)
so a legitimately long-running update isn't mistaken for an
abandoned one mid-flight.
- Reject archive entries that could escape PATH_ROOT during extraction
(Zip Slip / CWE-22, CVE-2026-16767 in nelexa/zip's Windows path
handling) before calling extractTo(). Extracted the check into a
pure, unit-tested Update::isSafeArchiveEntry().
- Test helpers now use Symfony's Filesystem instead of native
file_exists()/touch()/unlink(), and cover the exact 600-second
staleness boundary.
Not changed: load.php's isCoreUpdateLockActive() still treats a
concurrent request that checks the lock a moment before an updater
creates it as unlocked. No amount of locking on the updater's side
closes that gap without every request re-checking continuously through
its whole lifecycle, which isn't how PHP's bootstrap works; the lock
already collapses the risk window from the full extraction+patch
duration down to that narrow edge.
* Add nelexa to cspell word list
The Zip Slip guard's docblock references the nelexa/zip package by
name, which cspell flagged as an unknown word.
* Address CodeRabbit review: Windows dot-space traversal, flaky test
- isSafeArchiveEntry() rejected a segment only if it matched '..'
exactly. Windows strips trailing spaces and periods from path
components, so an entry like '.. .\outside.php' resolved to '..' at
extraction time on that platform without matching the literal
string. Reject any segment that is nothing but dots/spaces once
trimmed, which also subsumes the old exact-'..' check. Added
regression tests for the dot-space and all-dots cases, plus a
positive test that legitimate filenames with spaces/periods still
pass.
- isCoreUpdateLockActive() took no arguments and called time()
internally, so the exact-600-second boundary test set the lock's
mtime from one time() call and asserted against a second, separate
time() call a moment later - a clock tick between them could flip a
correctly-600-second-old lock to 601 and fail the test against
correct production code. Added an optional $now parameter
(defaulting to time(), so every other caller is unaffected) and had
the test capture one reference time for both the mtime and the
assertion.
- Added "autoloading"/"autoloaded" to cspell.json - load.php used
both already, but no CI run had actually spellchecked that file yet
(confirmed via the GitHub API: its check-run shows "skipped" on the
commit that introduced them), so this would have surfaced as an
unrelated failure later.
