diff --git a/system/src/Grav/Common/Upgrade/SafeUpgradeService.php b/system/src/Grav/Common/Upgrade/SafeUpgradeService.php index b3b768dab..c3a2843cc 100644 --- a/system/src/Grav/Common/Upgrade/SafeUpgradeService.php +++ b/system/src/Grav/Common/Upgrade/SafeUpgradeService.php @@ -23,24 +23,18 @@ use RecursiveIteratorIterator; use FilesystemIterator; use function array_key_exists; use function basename; -use function chgrp; use function chmod; -use function chown; use function copy; use function count; use function dirname; use function file_get_contents; use function file_put_contents; -use function filegroup; -use function fileowner; use function glob; use function in_array; use function is_dir; use function is_file; use function json_decode; use function json_encode; -use function lchgrp; -use function lchown; use function preg_match; use function preg_replace; use function property_exists; @@ -54,8 +48,6 @@ use function trim; use function uniqid; use function unlink; use function ltrim; -use function posix_getgrgid; -use function posix_getpwuid; use const GRAV_ROOT; use const GLOB_ONLYDIR; use const JSON_PRETTY_PRINT; @@ -74,7 +66,7 @@ class SafeUpgradeService * * @var string */ - public const IMPLEMENTATION_VERSION = '20251106'; // 2025-11-06 - Added preflight to Install.php + public const IMPLEMENTATION_VERSION = '20251109'; // 2025-11-09 - Simplified to match traditional upgrade /** @var string */ private $rootPath; @@ -100,8 +92,6 @@ class SafeUpgradeService ]; /** @var callable|null */ private $progressCallback = null; - /** @var int */ - private $metadataWarningCount = 0; /** * @param array $options @@ -428,7 +418,9 @@ class SafeUpgradeService } $destination = $targetBase . DIRECTORY_SEPARATOR . $entry; - $metadata = $this->captureEntryMeta($destination); + + // Use the same simple approach as traditional upgrade: + // Delete old, copy new, let filesystem handle ownership $this->removeEntry($destination); if (is_link($source)) { @@ -436,29 +428,25 @@ class SafeUpgradeService if (!@symlink(readlink($source), $destination)) { throw new RuntimeException(sprintf('Failed to replicate symlink "%s".', $source)); } - $this->applyEntryMeta($destination, $metadata); - continue; } elseif (is_dir($source)) { Folder::create(dirname($destination)); - Folder::rcopy($source, $destination, true); - $this->applyEntryMeta($destination, $metadata); - continue; + // DON'T preserve permissions - let filesystem inherit from parent + // This matches traditional upgrade behavior + Folder::rcopy($source, $destination, false); + + // Set bin/ permissions like traditional upgrade does + if ($entry === 'bin') { + $binFiles = glob($destination . DIRECTORY_SEPARATOR . '*') ?: []; + foreach ($binFiles as $binFile) { + @chmod($binFile, 0755); + } + } } else { Folder::create(dirname($destination)); if (!@copy($source, $destination)) { throw new RuntimeException(sprintf('Failed to copy file "%s" to "%s".', $source, $destination)); } - $perm = @fileperms($source); - if ($perm !== false) { - @chmod($destination, $perm & 0777); - } - $mtime = @filemtime($source); - if ($mtime !== false) { - @touch($destination, $mtime); - } } - - $this->applyEntryMeta($destination, $metadata); } } @@ -471,172 +459,6 @@ class SafeUpgradeService } } - /** - * Capture ownership and permission data for an existing filesystem entry. - * - * @param string $path - * @return array - */ - private function captureEntryMeta(string $path): array - { - if (!file_exists($path) && !is_link($path)) { - return []; - } - - $meta = [ - 'link' => is_link($path), - ]; - - $perms = @fileperms($path); - if ($perms !== false) { - $meta['perms'] = $perms & 0777; - } - - $owner = @fileowner($path); - if ($owner !== false) { - $meta['owner'] = $owner; - } - - $group = @filegroup($path); - if ($group !== false) { - $meta['group'] = $group; - } - - return $meta; - } - - /** - * Reapply ownership and permission data to a copied entry when possible. - * - * @param string $path - * @param array $meta - * @return void - */ - private function applyEntryMeta(string $path, array $meta): void - { - if (!$meta) { - return; - } - - if (isset($meta['perms'])) { - $result = @chmod($path, (int) $meta['perms']); - if ($result === false) { - $this->logMetadataWarning('chmod', $path, $meta['perms']); - } - } - - $isLink = !empty($meta['link']); - - if (isset($meta['owner'])) { - $owner = $this->resolveOwner($meta['owner']); - $result = false; - if ($isLink && function_exists('lchown')) { - $result = @lchown($path, $owner); - } elseif (!$isLink && function_exists('chown')) { - $result = @chown($path, $owner); - } - if ($result === false) { - $this->logMetadataWarning('chown', $path, $owner); - } - } - - if (isset($meta['group'])) { - $group = $this->resolveGroup($meta['group']); - $result = false; - if ($isLink && function_exists('lchgrp')) { - $result = @lchgrp($path, $group); - } elseif (!$isLink && function_exists('chgrp')) { - $result = @chgrp($path, $group); - } - if ($result === false) { - $this->logMetadataWarning('chgrp', $path, $group); - } - } - } - - /** - * Log a warning when metadata operations fail. - * - * @param string $operation Operation that failed (chmod, chown, chgrp) - * @param string $path Path to the file/directory - * @param mixed $value Value that was attempted (permissions, owner, group) - * @return void - */ - private function logMetadataWarning(string $operation, string $path, $value): void - { - $this->metadataWarningCount++; - - // Try to get Grav logger if available - try { - $grav = Grav::instance(); - if (isset($grav['log'])) { - $grav['log']->warning(sprintf( - 'Safe-upgrade: Failed to apply %s(%s, %s). File permissions/ownership may not be preserved correctly. ' . - 'This is usually not critical but may require manual permission fixes after upgrade.', - $operation, - $path, - is_scalar($value) ? $value : gettype($value) - )); - } - } catch (\Throwable $e) { - // Silently continue if logging fails - don't break the upgrade - } - } - - /** - * Get count of metadata warnings during upgrade. - * - * @return int - */ - public function getMetadataWarningCount(): int - { - return $this->metadataWarningCount; - } - - /** - * Resolve stored owner identifier to a format accepted by chown/lchown. - * - * @param int|string $owner - * @return int|string - */ - private function resolveOwner($owner) - { - if (is_string($owner)) { - return $owner; - } - - if (function_exists('posix_getpwuid')) { - $info = @posix_getpwuid((int) $owner); - if (is_array($info) && isset($info['name'])) { - return $info['name']; - } - } - - return (int) $owner; - } - - /** - * Resolve stored group identifier to a format accepted by chgrp/lchgrp. - * - * @param int|string $group - * @return int|string - */ - private function resolveGroup($group) - { - if (is_string($group)) { - return $group; - } - - if (function_exists('posix_getgrgid')) { - $info = @posix_getgrgid((int) $group); - if (is_array($info) && isset($info['name'])) { - return $info['name']; - } - } - - return (int) $group; - } - public function setProgressCallback(?callable $callback): self { $this->progressCallback = $callback; diff --git a/system/src/Grav/Installer/Install.php b/system/src/Grav/Installer/Install.php index 0c88f515d..b1e4556fc 100644 --- a/system/src/Grav/Installer/Install.php +++ b/system/src/Grav/Installer/Install.php @@ -459,19 +459,7 @@ ERR; */ private function shouldUseSafeUpgrade(): bool { - // CRITICAL: Check if class exists WITHOUT triggering autoloader - // If not loaded yet, manually load the NEW one from this package - if (!class_exists('Grav\\Common\\Upgrade\\SafeUpgradeService', false)) { - // Class not loaded yet - try to load from NEW package - $serviceFile = $this->location . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; - if (!file_exists($serviceFile)) { - return false; // SafeUpgradeService not available in this package - } - // Load the NEW SafeUpgradeService from this package - require_once $serviceFile; - } - - // Check static override first + // Check static override first (for programmatic control) if (null !== self::$forceSafeUpgrade) { return self::$forceSafeUpgrade; } @@ -482,17 +470,60 @@ ERR; return $envValue === '1'; } - // Check Grav config + // CRITICAL CHECK: Ensure current installation supports SafeUpgradeService + // This must be checked BEFORE reading config, because the NEW package's system.yaml + // has safe_upgrade:true as default, which would be returned even for old installations + $currentServiceFile = GRAV_ROOT . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; + if (!file_exists($currentServiceFile)) { + // Current installation doesn't have SafeUpgradeService (upgrading from < 1.7.50) + // Use traditional upgrade method regardless of config + return false; + } + + // PRIMARY CHECK: Check Grav config setting + // Only use safe-upgrade if explicitly enabled in user configuration try { $grav = Grav::instance(); if ($grav && isset($grav['config'])) { - return (bool) $grav['config']->get('system.updates.safe_upgrade', true); + // IMPORTANT: Read from USER config only, not merged config + // This prevents the NEW package's default (true) from being used + $userConfigFile = USER_DIR . 'config/system.yaml'; + if (file_exists($userConfigFile)) { + $userConfig = \Grav\Common\Yaml::parseFile($userConfigFile); + if (is_array($userConfig) && isset($userConfig['updates']['safe_upgrade'])) { + // User has explicitly set this in their config + return (bool) $userConfig['updates']['safe_upgrade']; + } + } + + // Fallback: try reading from merged config + // This is safe now because we already verified current installation has SafeUpgradeService + $configValue = $grav['config']->get('system.updates.safe_upgrade'); + if ($configValue !== null) { + return (bool) $configValue; + } } } catch (\Throwable $e) { - // Grav container may not be initialised yet, default to safe upgrade. + // Grav container may not be initialised yet + // Fall through to default behavior } - return true; + // FINAL STEP: Load SafeUpgradeService from NEW package if needed + // Only reached if current installation HAS SafeUpgradeService but config is not set + if (!class_exists('Grav\\Common\\Upgrade\\SafeUpgradeService', false)) { + // Class not loaded yet - try to load from NEW package + $serviceFile = $this->location . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; + if (!file_exists($serviceFile)) { + return false; // SafeUpgradeService not available in this package + } + // Load the NEW SafeUpgradeService from this package + require_once $serviceFile; + } + + // If we get here: current installation HAS SafeUpgradeService, but config is not explicitly set + // Default to FALSE (traditional upgrade) for safety + // Users must explicitly enable system.updates.safe_upgrade to use it + return false; } /**