From 228d916d5b86ca0f8f32922b17e0a261335a021b Mon Sep 17 00:00:00 2001 From: "Beau Beauchamp, WebTigers" Date: Thu, 24 Sep 2026 09:22:55 -0400 Subject: [PATCH] Atomic + permission-safe self-update (TIGER-225) A web-driven core update that failed part-way white-screened a panel: Composer, running as apache, could not delete a vendor file owned by another user and aborted mid-extract, leaving vendor/webtigers/tiger-core half-removed (functions.php gone) -> fatal on autoload -> every request 500. Tiger_Update_Composer::update() is now atomic and fails clean: - DEEP writability preflight: walk the whole vendor tree (is_writable only checked the top dir before) and abort BEFORE any change if the web user can't replace a nested dir, naming it + a fix command. Deletion needs write on the containing dir, so we test dirs. - Stage the target package ASIDE (rename to var/update-rollback-*) before Composer runs. This is the rollback point AND removes the failure mode: with the old dir gone, Composer does a clean fresh install with nothing to delete. - Restore-on-failure: if Composer errors, or "succeeds" but the package is missing/incomplete (tiger-core must have Version.php + functions.php), move the previous version back. A failed update never leaves the site broken. Version 1.15.1; CHANGELOG. 4 new unit tests for the deep preflight, the intact check, and restore (11 pass). Note: the full release-swap distribution model (pre-vendored bundle + symlink flip; never run Composer on a live box) remains the longer-term direction in TIGER-225; this hardens the in-place path meanwhile. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01ASauLLscjqdsNqBNsx2Typ --- CHANGELOG.md | 12 +++ library/Tiger/Update/Composer.php | 119 ++++++++++++++++++++++++++++- library/Tiger/Version.php | 2 +- tests/Unit/Update/ComposerTest.php | 71 ++++++++++++++++- 4 files changed, 199 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 937dac73..d29cac6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ All notable changes to **Tiger Core** (`webtigers/tiger-core`). Format follows ## [Unreleased] +## [1.15.1] — 2026-09-24 + +### Fixed +- **Self-update is now atomic + permission-safe** (`Tiger_Update_Composer`). A web-driven core update + that failed part-way (Composer could not delete a vendor file the web user didn't own) previously left + `vendor/webtigers/tiger-core` half-removed → a fatal on autoload → the whole site 500'd. Now: a **deep + writability preflight** walks the vendor tree and aborts cleanly *before* any change if the web user + can't replace it (naming the offending dir + a fix command); the target package is **staged aside** as a + rollback point before Composer runs (so Composer does a clean fresh install with nothing to delete); and + on any failure — or a "success" that left the package missing/incomplete — the previous version is + **restored**, so a failed update never breaks the site. (TIGER-225.) + ## [1.15.0] — 2026-09-23 ### Added diff --git a/library/Tiger/Update/Composer.php b/library/Tiger/Update/Composer.php index 2caa50be..ebf00cd8 100644 --- a/library/Tiger/Update/Composer.php +++ b/library/Tiger/Update/Composer.php @@ -63,9 +63,22 @@ public static function update(array $opts) } $add('preflight', true, 'Composer runnable, composer.json present, vendor/ writable.'); - $verFile = $root . '/vendor/' . $package . '/library/Tiger/Version.php'; // tiger-core layout + $pkgDir = $root . '/vendor/' . $package; + $verFile = $pkgDir . '/library/Tiger/Version.php'; // tiger-core layout $before = self::_versionIn($verFile); + // DEEP writability preflight. is_writable(vendor) above only checks the TOP dir; the real-world + // failure (which white-screened a panel) is a NESTED dir the web user doesn't own — deleting a + // file needs write on its CONTAINING dir, so Composer aborts part-way and leaves a half-installed + // package. Catch it here and fail CLEAN, before anything is touched. + $unwritable = self::_firstUnwritableDir($root . '/vendor'); + if ($unwritable !== null) { + return $fail('Update aborted — nothing was changed. "' . $unwritable . '" is not writable by the ' + . 'web user (' . self::_procUser() . '), so Composer would fail part-way and break the install. ' + . 'Make the vendor tree writable by the web user (e.g. `chown -R :' . self::_procGroup() + . ' vendor && find vendor -type d -exec chmod 2775 {} +`), then retry.'); + } + // A writable HOME/COMPOSER_HOME (web users often have none), unbounded memory, no TTY. $composerHome = $root . '/var/composer-home'; @mkdir($composerHome, 0775, true); @@ -79,6 +92,20 @@ public static function update(array $opts) // log). Seed our HOME's gitconfig to trust any path so the update log stays clean. @file_put_contents($composerHome . '/.gitconfig', "[safe]\n\tdirectory = *\n"); + // ATOMIC: stage the current package ASIDE as a rollback point BEFORE Composer runs. This also + // removes the exact failure mode above — with the old dir gone, Composer does a clean fresh + // INSTALL into an empty slot (nothing to delete). Any failure below restores it, so the site is + // never left half-updated. + $backup = null; + if (is_dir($pkgDir)) { + $backup = $root . '/var/update-rollback-' . preg_replace('/[^a-z0-9]+/i', '-', $package) . '-' . time(); + @mkdir(dirname($backup), 0775, true); + if (!@rename($pkgDir, $backup)) { + return $fail('Update aborted — nothing was changed. Could not stage a rollback of ' . $package + . ' (rename failed); check that ' . $root . '/var is writable by the web user.'); + } + } + // --with-all-dependencies so a required tigerzf/polyfill bump comes along; --no-dev for a // production-shaped tree; --no-scripts so a post-update hook can't fail the update mid-request. $cmd = $binary . ' update ' . escapeshellarg($package) @@ -86,9 +113,19 @@ public static function update(array $opts) list($code, $out) = self::_run($cmd, $root, self::TIMEOUT); $tail = self::_tail($out, 4000); - if ($code !== 0) { - return $fail("Composer exited with code {$code}." . ($tail !== '' ? "\n" . $tail : '')); + // Restore-on-failure: Composer errored, OR it "succeeded" but the package is missing/incomplete + // (the half-extracted state). Either way, put the previous version back so the site stays up. + if ($code !== 0 || !self::_packageIntact($pkgDir, $verFile)) { + $restored = self::_restore($pkgDir, $backup); + $why = ($code !== 0) + ? "Composer exited with code {$code}." + : 'Composer finished but ' . $package . ' is missing or incomplete on disk.'; + return $fail($why . ($restored ? ' Rolled back to the previous version — the site is unchanged.' : '') + . ($tail !== '' ? "\n" . $tail : '')); } + + // Success — drop the rollback copy. + if ($backup !== null) { self::_rmrf($backup); } $add('composer', true, 'composer update ' . $package . ' finished.' . ($tail !== '' ? "\n" . $tail : '')); // Re-read from disk — the running process still holds the old Version constant. @@ -100,6 +137,82 @@ public static function update(array $opts) return ['ok' => true, 'version' => $after, 'log' => $log]; } + // ---- atomicity helpers ----------------------------------------------------- + + /** + * The first directory at/under $root that this process cannot write (so Composer could not delete a + * file in it), or null if the whole tree is writable. Deletion needs write on the CONTAINING dir, so + * we test dirs, not files. Symlinked dirs are skipped (not ours to chmod). + */ + protected static function _firstUnwritableDir($root) + { + if (!is_dir($root)) { return null; } + if (!is_writable($root)) { return $root; } + try { + $it = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::SELF_FIRST); + foreach ($it as $path) { + if ($path->isDir() && !$path->isLink() && !is_writable((string) $path)) { + return (string) $path; + } + } + } catch (Exception $e) { + return null; // never let the check itself block an update + } + return null; + } + + /** Whether the package is fully installed on disk (dir + composer.json; +Version/functions for tiger-core). */ + protected static function _packageIntact($pkgDir, $verFile) + { + if (!is_dir($pkgDir) || !is_file($pkgDir . '/composer.json')) { return false; } + if (basename($pkgDir) === 'tiger-core') { // the classic half-extracted fatal is a missing one of these + return is_file($verFile) && is_file($pkgDir . '/functions.php'); + } + return true; + } + + /** Restore the staged rollback over a (possibly partial) package dir. Returns whether it was restored. */ + protected static function _restore($pkgDir, $backup) + { + if ($backup === null || !is_dir($backup)) { return false; } + self::_rmrf($pkgDir); + return @rename($backup, $pkgDir); + } + + /** Recursively remove a path. */ + protected static function _rmrf($path) + { + if ($path === '' || !file_exists($path)) { return; } + if (is_file($path) || is_link($path)) { @unlink($path); return; } + try { + $it = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST); + foreach ($it as $p) { ($p->isDir() && !$p->isLink()) ? @rmdir((string) $p) : @unlink((string) $p); } + } catch (Exception $e) { /* best effort */ } + @rmdir($path); + } + + protected static function _procUser() + { + if (function_exists('posix_geteuid') && function_exists('posix_getpwuid')) { + $u = @posix_getpwuid(@posix_geteuid()); + if (is_array($u) && !empty($u['name'])) { return $u['name']; } + } + return get_current_user() ?: 'the web user'; + } + + protected static function _procGroup() + { + if (function_exists('posix_getegid') && function_exists('posix_getgrgid')) { + $g = @posix_getgrgid(@posix_getegid()); + if (is_array($g) && !empty($g['name'])) { return $g['name']; } + } + return 'apache'; + } + // ---- helpers --------------------------------------------------------------- /** Run a command, capturing merged output, with a wall-clock timeout. Returns [exitCode, output]. */ diff --git a/library/Tiger/Version.php b/library/Tiger/Version.php index 1b308150..90738d8d 100644 --- a/library/Tiger/Version.php +++ b/library/Tiger/Version.php @@ -9,5 +9,5 @@ class Tiger_Version { /** Current Tiger Core version. Keep in lockstep with the git tag cut for a release. */ - const VERSION = '1.15.0'; + const VERSION = '1.15.1'; } diff --git a/tests/Unit/Update/ComposerTest.php b/tests/Unit/Update/ComposerTest.php index 0f569bac..9a88c12b 100644 --- a/tests/Unit/Update/ComposerTest.php +++ b/tests/Unit/Update/ComposerTest.php @@ -94,12 +94,81 @@ public function tail_truncates_only_when_longer_than_the_limit(): void $this->assertStringStartsWith('…', $tail); $this->assertSame(11, mb_strlen($tail), 'the ellipsis + the last 10 chars'); } + + // ---- atomicity helpers (the deep preflight + rollback that keep a failed update from breaking the site) + + #[Test] + public function first_unwritable_dir_is_null_when_the_whole_tree_is_writable(): void + { + @mkdir($this->tmp . '/a/b', 0775, true); + $this->assertNull(UpdateComposerProbe::firstUnwritableDir($this->tmp)); + $this->assertNull(UpdateComposerProbe::firstUnwritableDir($this->tmp . '/does-not-exist')); + } + + #[Test] + public function first_unwritable_dir_finds_a_locked_nested_dir(): void + { + if (function_exists('posix_geteuid') && posix_geteuid() === 0) { + $this->markTestSkipped('root ignores permission bits, so the unwritable check cannot be exercised as root'); + } + @mkdir($this->tmp . '/pkg/.github/workflows', 0775, true); + @chmod($this->tmp . '/pkg/.github/workflows', 0500); // r-x: cannot delete files in it (the real failure) + $hit = UpdateComposerProbe::firstUnwritableDir($this->tmp); + $this->assertSame($this->tmp . '/pkg/.github/workflows', $hit); + @chmod($this->tmp . '/pkg/.github/workflows', 0775); // let tearDown clean it + } + + #[Test] + public function package_intact_requires_a_composer_json(): void + { + $pkg = $this->tmp . '/somepkg'; + @mkdir($pkg, 0775, true); + $this->assertFalse(UpdateComposerProbe::packageIntact($pkg, $pkg . '/library/Tiger/Version.php')); + file_put_contents($pkg . '/composer.json', '{}'); + $this->assertTrue(UpdateComposerProbe::packageIntact($pkg, $pkg . '/library/Tiger/Version.php')); + } + + #[Test] + public function tiger_core_intact_needs_version_and_functions(): void + { + $pkg = $this->tmp . '/tiger-core'; + @mkdir($pkg . '/library/Tiger', 0775, true); + file_put_contents($pkg . '/composer.json', '{}'); + $ver = $pkg . '/library/Tiger/Version.php'; + // composer.json alone is NOT enough for tiger-core (the half-extracted fatal is a missing functions.php) + $this->assertFalse(UpdateComposerProbe::packageIntact($pkg, $ver)); + file_put_contents($ver, "assertFalse(UpdateComposerProbe::packageIntact($pkg, $ver)); + file_put_contents($pkg . '/functions.php', "assertTrue(UpdateComposerProbe::packageIntact($pkg, $ver)); + } + + #[Test] + public function restore_puts_the_backup_back_over_a_broken_dir(): void + { + $pkg = $this->tmp . '/tiger-core'; + $backup = $this->tmp . '/rollback'; + @mkdir($backup, 0775, true); + file_put_contents($backup . '/functions.php', "assertTrue(UpdateComposerProbe::restore($pkg, $backup)); + $this->assertFileExists($pkg . '/functions.php'); + $this->assertFileDoesNotExist($pkg . '/partial.tmp'); // the broken tree was replaced + $this->assertDirectoryDoesNotExist($backup); // backup consumed by the rename + $this->assertFalse(UpdateComposerProbe::restore($pkg, null)); // nothing to restore + } } -/** Test seam: expose Tiger_Update_Composer's protected process/parse helpers (never a real update). */ +/** Test seam: expose Tiger_Update_Composer's protected process/parse/atomicity helpers (never a real update). */ final class UpdateComposerProbe extends Tiger_Update_Composer { public static function run($cmd, $cwd, $to): array { return self::_run($cmd, $cwd, $to); } public static function versionIn($f) { return self::_versionIn($f); } public static function tail($s, $m): string { return self::_tail($s, $m); } + public static function firstUnwritableDir($d) { return self::_firstUnwritableDir($d); } + public static function packageIntact($p, $v): bool { return self::_packageIntact($p, $v); } + public static function restore($p, $b): bool { return self::_restore($p, $b); } }