diff --git a/core/src/Console/SiteUpdateCommand.php b/core/src/Console/SiteUpdateCommand.php index 4cf8bf37c5..32d1b5f978 100644 --- a/core/src/Console/SiteUpdateCommand.php +++ b/core/src/Console/SiteUpdateCommand.php @@ -708,7 +708,7 @@ protected function composerBinaryCommand(): string } foreach ($this->composerBinaryCandidates() as $candidate) { - if (is_file($candidate) && is_executable($candidate)) { + if ($this->isExecutableFile($candidate)) { return escapeshellarg($candidate); } } @@ -716,6 +716,27 @@ protected function composerBinaryCommand(): string return 'composer'; } + /** + * Check whether a path is something the shell can run. + * + * On Windows is_executable() answers false even for a genuine + * composer.bat — it does not consult PATHEXT the way the shell does — so + * every candidate would be rejected no matter which paths were offered. + * There the file existing is the only signal available. + * + * @since 3.5.8 + * @param string $path Absolute path to test. + * @return bool + */ + protected function isExecutableFile(string $path): bool + { + if (!is_file($path)) { + return false; + } + + return windows_os() ? true : is_executable($path); + } + /** * Build fallback Composer executable candidates. * @@ -733,9 +754,55 @@ protected function composerBinaryCandidates(): array $candidates[] = $home . '/.composer/composer'; } + // Appended rather than switched on the platform. Every candidate is + // filtered by isExecutableFile() anyway, so an entry that cannot exist + // here costs one is_file() call, while a platform branch would be a + // new way to guess wrong — under WSL, or wherever the environment does + // not match what PHP_OS_FAMILY suggests. + $candidates = array_merge($candidates, $this->windowsComposerBinaryCandidates()); + return array_values(array_unique($candidates)); } + /** + * Build fallback Composer executable candidates for Windows layouts. + * + * The POSIX list finds nothing here: there is no /usr/local/bin, and a + * per-user install puts a shim in %APPDATA%\Composer rather than in a + * ~/.composer/composer file. Only shell-runnable shims are listed — + * composer.phar is deliberately absent, because it needs `php` in front of + * it and this list feeds a command that is executed directly. + * + * @since 3.5.8 + * @return array + */ + protected function windowsComposerBinaryCandidates(): array + { + $candidates = []; + + // Where the Composer-Setup installer puts a machine-wide install. + $programData = trim((string) getenv('ProgramData')); + if ($programData !== '') { + $base = rtrim(str_replace('\\', '/', $programData), '/') . '/ComposerSetup/bin/composer'; + $candidates[] = $base . '.bat'; + $candidates[] = $base . '.exe'; + } + + // A per-user install. + $appData = trim((string) getenv('APPDATA')); + if ($appData !== '') { + $base = rtrim(str_replace('\\', '/', $appData), '/') . '/Composer/composer'; + $candidates[] = $base . '.bat'; + $candidates[] = $base . '.exe'; + } + + foreach ($this->homeDirectories() as $home) { + $candidates[] = $home . '/AppData/Roaming/Composer/composer.bat'; + } + + return array_values(array_unique(array_filter($candidates))); + } + /** * Resolve possible home directories without relying on shell "~" expansion. * @@ -775,7 +842,16 @@ protected function shellCommandExists(string $command): bool $output = []; $exitCode = 1; - exec('command -v ' . escapeshellarg($command) . ' >/dev/null 2>&1', $output, $exitCode); + // `command -v` is a POSIX shell builtin and /dev/null is a POSIX + // device; cmd.exe has neither, so on Windows this probe reported "not + // found" for every command — including ones plainly on PATH — and the + // resolver fell through to candidate paths that do not exist there + // either. `where` is the native equivalent and answers 0 when found. + $probe = windows_os() + ? 'where ' . escapeshellarg($command) . ' >NUL 2>NUL' + : 'command -v ' . escapeshellarg($command) . ' >/dev/null 2>&1'; + + exec($probe, $output, $exitCode); return (int) $exitCode === 0; } diff --git a/core/src/Console/SystemTasks/TaskWorkerCommand.php b/core/src/Console/SystemTasks/TaskWorkerCommand.php index a3925acd21..a9145e19d0 100644 --- a/core/src/Console/SystemTasks/TaskWorkerCommand.php +++ b/core/src/Console/SystemTasks/TaskWorkerCommand.php @@ -1,9 +1,7 @@ markPick($host, $pid); try { - switch ((string) $task->type) { - case 'console_install': - $flow = new ConsoleInstallFlowService(); - $result = $flow->execute($task, function ($step, $progress, $message, $level = 'info', array $context = []) use (&$task, $taskService) { - $task = $taskService->updateTaskProgress( - $task, - 'running', - (int) $progress, - (string) $step, - (string) $message, - (string) $level, - $context - ); - }); - - $taskService->markTaskSucceeded( - $task, - isset($result['message']) ? (string) $result['message'] : 'System task completed successfully.', - isset($result['result']) && is_array($result['result']) ? $result['result'] : [] - ); - $workerHealth->markSuccess($host, $pid); - $this->info('[system:task-worker] console install task completed'); - return self::SUCCESS; - - case 'console_uninstall': - $flow = new ConsoleUninstallFlowService(); - $result = $flow->execute($task, function ($step, $progress, $message, $level = 'info', array $context = []) use (&$task, $taskService) { - $task = $taskService->updateTaskProgress( - $task, - 'running', - (int) $progress, - (string) $step, - (string) $message, - (string) $level, - $context - ); - }); - - $taskService->markTaskSucceeded( - $task, - isset($result['message']) ? (string) $result['message'] : 'System task completed successfully.', - isset($result['result']) && is_array($result['result']) ? $result['result'] : [] - ); - $workerHealth->markSuccess($host, $pid); - $this->info('[system:task-worker] console uninstall task completed'); - return self::SUCCESS; - - case 'site_update': - $flow = new SiteUpdateFlowService(); - $result = $flow->execute($task, function ($step, $progress, $message, $level = 'info', array $context = []) use (&$task, $taskService) { - $task = $taskService->updateTaskProgress( - $task, - 'running', - (int) $progress, - (string) $step, - (string) $message, - (string) $level, - $context - ); - }); - - $taskService->markTaskSucceeded( - $task, - isset($result['message']) ? (string) $result['message'] : 'Site update completed successfully.', - isset($result['result']) && is_array($result['result']) ? $result['result'] : [] - ); - $workerHealth->markSuccess($host, $pid); - $this->info('[system:task-worker] site update task completed'); - return self::SUCCESS; - - default: - $taskService->markTaskFailed( - $task, - 'TASK_TYPE_NOT_ALLOWED', - 'Unsupported system task type for this worker.' - ); - $workerHealth->markFailure('TASK_TYPE_NOT_ALLOWED', $host, $pid); - Log::warning('[system:task-worker] unsupported task type', [ - 'task_id' => (int) $task->id, - 'type' => (string) $task->type, - ]); - $this->warn('[system:task-worker] unsupported task type'); - return self::SUCCESS; + $type = (string) $task->type; + if (!SystemTaskRegistry::has($type)) { + $taskService->markTaskFailed( + $task, + 'TASK_TYPE_NOT_ALLOWED', + 'Unsupported system task type for this worker.' + ); + $workerHealth->markFailure('TASK_TYPE_NOT_ALLOWED', $host, $pid); + Log::warning('[system:task-worker] unsupported task type', [ + 'task_id' => (int) $task->id, + 'type' => $type, + ]); + $this->warn('[system:task-worker] unsupported task type'); + return self::SUCCESS; } + + $handler = SystemTaskRegistry::handler($type); + $result = $handler->execute($task, function ($step, $progress, $message, $level = 'info', array $context = []) use (&$task, $taskService) { + $task = $taskService->updateTaskProgress( + $task, + 'running', + (int) $progress, + (string) $step, + (string) $message, + (string) $level, + $context + ); + }); + + $taskService->markTaskSucceeded( + $task, + isset($result['message']) ? (string) $result['message'] : SystemTaskRegistry::label($type) . ' completed successfully.', + isset($result['result']) && is_array($result['result']) ? $result['result'] : [] + ); + $workerHealth->markSuccess($host, $pid); + $this->info('[system:task-worker] ' . $type . ' task completed'); + return self::SUCCESS; } catch (\Throwable $exception) { $errorCode = 'TASK_EXECUTION_FAILED'; $taskService->markTaskFailed($task, $errorCode, $exception->getMessage(), [ diff --git a/core/src/Core.php b/core/src/Core.php index 70441c9963..b5feb885d3 100644 --- a/core/src/Core.php +++ b/core/src/Core.php @@ -87,6 +87,24 @@ class Core extends AbstractLaravel implements Interfaces\CoreInterface public $documentOutput; public $tstart = 0; public $mstart = 0; + /** + * Conditional-tag commands referenced by index from the source generated in + * mergeConditionalTagsContent(), so they never have to be quoted into it. + * + * @var array + */ + private $ctagCmds = []; + + /** + * Extensions @FILE refuses to serve. `.php` alone left `.phtml`/`.php5`/`.inc` readable as + * plain text, which discloses their source. + * + * @var string[] + */ + private const AT_BIND_FILE_DENIED_EXTENSIONS = [ + '.php', '.php3', '.php4', '.php5', '.php7', '.php8', '.phps', '.phtml', '.phar', '.inc', + ]; + public $minParserPasses = 2; public $maxParserPasses = 10; public $documentObject = []; @@ -1872,6 +1890,12 @@ public function mergeConditionalTagsContent( $content ); + // The command is handed to _parseCTagCMD() by index instead of being quoted into the + // generated source. Escaping it was never sufficient: a backslash in front of the quote + // consumed the escape and let the rest of the command close the string literal and run as + // PHP. Passing a reference removes the concatenation, so there is nothing left to escape. + $ctagOffset = count($this->ctagCmds); + $pieces = explode('<@IF:', $content); foreach ($pieces as $i => $split) { if ($i === 0) { @@ -1879,8 +1903,8 @@ public function mergeConditionalTagsContent( continue; } [$cmd, $text] = explode('>', $split, 2); - $cmd = str_replace("'", "\'", $cmd); - $content .= "_parseCTagCMD('" . $cmd . "')): ?>"; + $index = array_push($this->ctagCmds, $cmd) - 1; + $content .= '_parseCTagCMD($this->ctagCmds[' . $index . '])): ?>'; $content .= $text; } $pieces = explode('<@ELSEIF:', $content); @@ -1890,15 +1914,21 @@ public function mergeConditionalTagsContent( continue; } [$cmd, $text] = explode('>', $split, 2); - $cmd = str_replace("'", "\'", $cmd); - $content .= "_parseCTagCMD('" . $cmd . "')): ?>"; + $index = array_push($this->ctagCmds, $cmd) - 1; + $content .= '_parseCTagCMD($this->ctagCmds[' . $index . '])): ?>'; $content .= $text; } $content = str_replace(['<@ELSE>', '<@ENDIF>'], ['', ''], $content); ob_start(); - eval ('?>' . $content); - $content = ob_get_clean(); + try { + eval ('?>' . $content); + } finally { + $content = ob_get_clean(); + // A nested parse has already trimmed its own entries, so the indices baked into the + // source above stayed valid for the whole eval. + array_splice($this->ctagCmds, $ctagOffset); + } $content = str_replace( ["{$sp}h", "{$sp}p", "{$sp}s", "{$sp}e"], [''], @@ -2316,23 +2346,12 @@ public function _getSGVar($value) $this->setConfig('enable_filter', $_); $key = str_replace(['(', ')'], ["['", "']"], $key); $key = rtrim($key, ';'); - if (Str::contains($key, '$_SESSION')) { - $_ = $_SESSION; - $key = str_replace('$_SESSION', '$_', $key); - if (isset($_['mgrFormValues'])) { - unset($_['mgrFormValues']); - } - if (isset($_['token'])) { - unset($_['token']); - } - } - if (Str::contains($key, '[')) { - $value = $key ? eval ("return {$key};") : ''; - } elseif (0 < eval ("return count({$key});")) { - $value = eval ("return print_r({$key},true);"); - } else { - $value = ''; - } + + // The superglobal is read by walking the array, not by evaluating the tag. eval() only + // looked safe here because `(` and `)` were rewritten away, but PHP's backtick operator + // needs no parentheses, so `[[$_SERVER . `id` ]]` reached the shell. + $value = $this->resolveSGVar($key); + if ($modifiers !== false) { $value = $this->applyFilter($value, $modifiers, $key); } @@ -2340,6 +2359,83 @@ public function _getSGVar($value) return $value; } + /** + * Read one superglobal entry named by a parser tag. + * + * Accepts `$_GET(key)` and `$_GET['key']` (the former is rewritten into the latter by the + * caller), nested to any depth, plus the bare `$_SERVER` form that dumps the whole array. + * Anything else - arithmetic, concatenation, backticks - is refused rather than evaluated. + * + * @param string $key + * @return mixed + * @since 3.5.8 + */ + private function resolveSGVar($key) + { + if (!preg_match('@^\$_(GET|POST|SESSION|COOKIE|REQUEST|SERVER|FILES|ENV)@', $key, $matches)) { + return ''; + } + + $path = []; + $rest = substr($key, strlen($matches[0])); + while ($rest !== '' && $rest !== false) { + if (!preg_match('@^\[\s*([\'"]?)([^\[\]\'"]*)\1\s*\]@', $rest, $accessor)) { + // Trailing characters that are not an array access: refuse the whole tag. + return ''; + } + $path[] = $accessor[2]; + $rest = substr($rest, strlen($accessor[0])); + } + + $container = $this->getSuperGlobal($matches[1]); + + if ($path === []) { + return count($container) > 0 ? print_r($container, true) : ''; + } + + $cursor = $container; + foreach ($path as $segment) { + if (!is_array($cursor) || !array_key_exists($segment, $cursor)) { + return ''; + } + $cursor = $cursor[$segment]; + } + + return $cursor; + } + + /** + * @param string $name + * @return array + * @since 3.5.8 + */ + private function getSuperGlobal($name) + { + switch ($name) { + case 'GET': + return $_GET; + case 'POST': + return $_POST; + case 'COOKIE': + return $_COOKIE; + case 'REQUEST': + return $_REQUEST; + case 'SERVER': + return $_SERVER; + case 'FILES': + return $_FILES; + case 'ENV': + return $_ENV; + case 'SESSION': + $session = isset($_SESSION) && is_array($_SESSION) ? $_SESSION : []; + unset($session['mgrFormValues'], $session['token']); + + return $session; + } + + return []; + } + /** * @param $piece * @return null|string @@ -6300,6 +6396,47 @@ public function isSafeCode($phpcode = '', $safe_functions = '') * @param string $str * @return bool|mixed|string */ + /** + * Resolve one @FILE candidate to a real path inside the installation, or false. + * + * The old check compared the unresolved concatenation against EVO_MANAGER_PATH, so a `..` + * segment walked straight past it - and past EVO_BASE_PATH - to anywhere the web user could + * read. Containment is decided on the resolved path instead. + * + * @param string $candidate + * @return string|false + * @since 3.5.8 + */ + private function resolveAtBindFilePath($candidate) + { + $resolved = realpath($candidate); + if ($resolved === false || !is_file($resolved)) { + return false; + } + + $base = realpath(EVO_BASE_PATH); + if ($base === false) { + return false; + } + + $resolved = str_replace(DIRECTORY_SEPARATOR, '/', $resolved); + $base = rtrim(str_replace(DIRECTORY_SEPARATOR, '/', $base), '/') . '/'; + + if (strpos($resolved, $base) !== 0) { + return false; + } + + $manager = realpath(EVO_MANAGER_PATH); + if ($manager !== false) { + $manager = rtrim(str_replace(DIRECTORY_SEPARATOR, '/', $manager), '/') . '/'; + if (strpos($resolved, $manager) === 0) { + return false; + } + } + + return $resolved; + } + public function atBindFileContent($str = '') { @@ -6310,7 +6447,7 @@ public function atBindFileContent($str = '') $str = substr($str, 0, strpos("\n", $str)); } - if ($this->getExtFromFilename($str) === '.php') { + if (in_array($this->getExtFromFilename($str), self::AT_BIND_FILE_DENIED_EXTENSIONS, true)) { return 'Could not retrieve PHP file.'; } @@ -6325,16 +6462,11 @@ public function atBindFileContent($str = '') $search_path = ['assets/tvs/', 'assets/chunks/', 'assets/templates/', $this->getConfig('rb_base_url') . 'files/', '']; foreach ($search_path as $path) { - $file_path = EVO_BASE_PATH . $path . $str; - if (strpos($file_path, EVO_MANAGER_PATH) === 0) { - return $errorMsg; - } + $file_path = $this->resolveAtBindFilePath(EVO_BASE_PATH . $path . $str); - if (is_file($file_path)) { + if ($file_path !== false) { break; } - - $file_path = false; } if (!$file_path) { diff --git a/core/src/Interfaces/SystemTaskHandlerInterface.php b/core/src/Interfaces/SystemTaskHandlerInterface.php new file mode 100644 index 0000000000..5266892ce9 --- /dev/null +++ b/core/src/Interfaces/SystemTaskHandlerInterface.php @@ -0,0 +1,36 @@ +condition); - $isvalid = (int)(eval("return ({$conditional});")); + $isvalid = (int)ArithmeticExpression::evaluate($conditional); if ($isvalid) { return $this->srcValue; } @@ -487,7 +488,7 @@ public function getValueFromPreset($key, $value, $cmd, $opt) return null; case 'then': $conditional = implode(' ', $this->condition); - $isvalid = (int)eval("return ({$conditional});"); + $isvalid = (int)ArithmeticExpression::evaluate($conditional); if ($isvalid) { return $opt; } @@ -495,7 +496,7 @@ public function getValueFromPreset($key, $value, $cmd, $opt) return null; case 'else': $conditional = implode(' ', $this->condition); - $isvalid = (int)eval("return ({$conditional});"); + $isvalid = (int)ArithmeticExpression::evaluate($conditional); if (!$isvalid) { return $opt; } @@ -910,7 +911,9 @@ public function getValueFromPreset($key, $value, $cmd, $opt) } $filter = str_replace('?', $value, $filter); - return eval("return {$filter};"); + // The letter strip above leaves `$`, quotes and backslashes in place, which is + // enough to reach PHP through octal escapes; only arithmetic gets through now. + return ArithmeticExpression::evaluate($filter); case 'count': if ($value == '') { return 0; diff --git a/core/src/Legacy/Phx.php b/core/src/Legacy/Phx.php index 7f6917cf5f..a5c8617a5e 100644 --- a/core/src/Legacy/Phx.php +++ b/core/src/Legacy/Phx.php @@ -1,6 +1,7 @@ buildSqlHeader('--', (string) $database, $host)); - $command = 'PGPASSWORD=' . escapeshellarg($password) - . ' pg_dump --host ' . escapeshellarg($host) - . ' --username ' . escapeshellarg($username) - . ' --dbname ' . escapeshellarg((string) $database) - . ' --clean --inserts --no-owner --no-privileges >> ' . escapeshellarg((string) $tempFilePath); + $handle = fopen($tempFilePath, 'ab'); - exec($command, $output, $exitCode); + if ($handle === false) { + if (is_file($tempFilePath)) { + unlink($tempFilePath); + } + + return false; + } + + // No shell is involved here, and that is the point. The previous form + // was `PGPASSWORD=… pg_dump … >> file`, and a leading VAR=value + // assignment is POSIX shell syntax that cmd.exe rejects outright with + // "'PGPASSWORD' is not recognized", so this backup could never succeed + // on Windows. Passing the password as an environment entry and the + // arguments as a list works the same way on every platform, and has + // the side benefit that nothing has to be quoted for a shell. + $process = new Process( + [ + 'pg_dump', + '--host', $host, + '--username', $username, + '--dbname', (string) $database, + '--clean', + '--inserts', + '--no-owner', + '--no-privileges', + ], + null, + ['PGPASSWORD' => $password] + ); + $process->setTimeout(null); + + try { + // Streamed rather than buffered: a dump is as large as the + // database, and getOutput() would hold all of it in memory. The + // shell redirect this replaces streamed too, so buffering here + // would be a regression on exactly the databases worth backing up. + $process->run(static function ($type, $buffer) use ($handle) { + if ($type === Process::OUT) { + fwrite($handle, $buffer); + } + }); + } catch (\Throwable $exception) { + fclose($handle); + + if (is_file($tempFilePath)) { + unlink($tempFilePath); + } + + return false; + } + + fclose($handle); + clearstatcache(true, $tempFilePath); - if ((int) $exitCode !== 0 || !is_file($tempFilePath) || filesize($tempFilePath) <= 0) { + if (!$process->isSuccessful() || !is_file($tempFilePath) || filesize($tempFilePath) <= 0) { if (is_file($tempFilePath)) { unlink($tempFilePath); } diff --git a/core/src/Services/SystemTasks/ConsoleInstallFlowService.php b/core/src/Services/SystemTasks/ConsoleInstallFlowService.php index 9bbad444c7..1baceed04d 100644 --- a/core/src/Services/SystemTasks/ConsoleInstallFlowService.php +++ b/core/src/Services/SystemTasks/ConsoleInstallFlowService.php @@ -1,10 +1,11 @@ SystemTaskRegistry::MODE_CONCURRENT, + * 'parallelism' => 3, + * 'permissions' => ['aimage'], + * 'creator' => [ImageBatchHandler::class, 'queue'], + * ]); + * + * Two defaults are deliberately strict. An unregistered type is refused rather + * than attempted, so a package uninstalled while its work was queued fails + * those tasks with a clear code instead of leaving them to be picked forever. + * And a registration that does not say otherwise is *exclusive*, because the + * three built-in types are, and a caller that has not thought about + * concurrency should inherit the conservative answer. + */ +final class SystemTaskRegistry +{ + /** + * Only one exclusive task runs at a time, across every type. + * + * This is what console installs and site updates need: they rewrite files + * under the document root and run Composer, so a second one alongside is a + * corrupted install. + */ + public const MODE_EXCLUSIVE = 'exclusive'; + + /** + * Ordinary background work. Bounded by the type's own `parallelism`, and + * not blocked by — nor blocking — tasks of other types. + */ + public const MODE_CONCURRENT = 'concurrent'; + + /** The `type` column is varchar(64); a longer type would be silently truncated. */ + public const MAX_TYPE_LENGTH = 64; + + /** + * The types this class ships, which a package may never redefine. + * + * Allowing a re-registration here would let any package that boots first + * substitute its own handler for `site_update` — which runs Composer and + * rewrites the core — and inherit the super-admin gate that makes the real + * one safe. + */ + private const BUILT_IN = ['console_install', 'console_uninstall', 'site_update']; + + /** @var array */ + private static array $definitions = []; + + private static bool $defaultsRegistered = false; + + /** + * Declare a task type. + * + * @param string $type stored verbatim in `system_cli_tasks.type` + * @param class-string|SystemTaskHandlerInterface|callable $handler + * resolved lazily — a class string is not instantiated until a + * worker actually picks a task of this type + * @param array{ + * mode?: string, + * parallelism?: int, + * permissions?: string[], + * super_admin?: bool, + * creator?: callable, + * label?: string + * } $options + * + * @throws InvalidArgumentException on a malformed type, an unknown mode, or + * an attempt to redefine a built-in type + */ + public static function register(string $type, $handler, array $options = []): void + { + $type = trim($type); + + if ($type === '' || strlen($type) > self::MAX_TYPE_LENGTH) { + throw new InvalidArgumentException( + 'A system task type must be between 1 and ' . self::MAX_TYPE_LENGTH . ' characters.' + ); + } + + if (!preg_match('/^[a-z0-9][a-z0-9_.\-]*$/', $type)) { + throw new InvalidArgumentException( + 'A system task type may only contain lowercase letters, digits, underscore, dot and hyphen: ' . $type + ); + } + + self::registerDefaults(); + + if (in_array($type, self::BUILT_IN, true) && isset(self::$definitions[$type])) { + throw new InvalidArgumentException('The built-in system task type "' . $type . '" cannot be redefined.'); + } + + $mode = (string) ($options['mode'] ?? self::MODE_EXCLUSIVE); + + if (!in_array($mode, [self::MODE_EXCLUSIVE, self::MODE_CONCURRENT], true)) { + throw new InvalidArgumentException('Unknown system task mode "' . $mode . '" for type "' . $type . '".'); + } + + self::$definitions[$type] = [ + 'type' => $type, + 'handler' => $handler, + 'mode' => $mode, + // Parallelism is meaningless for an exclusive type, which is + // capped at one by the mode itself. Pinning it to 1 keeps callers + // from reading a number that does not apply. + 'parallelism' => $mode === self::MODE_CONCURRENT + ? max(1, (int) ($options['parallelism'] ?? 1)) + : 1, + 'permissions' => array_values(array_filter(array_map( + static fn ($permission) => trim((string) $permission), + (array) ($options['permissions'] ?? []) + ))), + 'super_admin' => (bool) ($options['super_admin'] ?? false), + 'creator' => isset($options['creator']) && is_callable($options['creator']) + ? $options['creator'] + : null, + 'label' => trim((string) ($options['label'] ?? $type)), + ]; + } + + /** + * Drop a registration. + * + * Exists for tests and for a package that unregisters itself on teardown. + * Built-in types cannot be removed — a queue that has forgotten how to run + * a site update is worse than one that refuses to forget. + */ + public static function forget(string $type): void + { + self::registerDefaults(); + + if (in_array($type, self::BUILT_IN, true)) { + return; + } + + unset(self::$definitions[$type]); + } + + public static function has(string $type): bool + { + self::registerDefaults(); + + return isset(self::$definitions[trim($type)]); + } + + /** @return array|null the whole definition, or null when the type is unknown */ + public static function definition(string $type): ?array + { + self::registerDefaults(); + + return self::$definitions[trim($type)] ?? null; + } + + /** @return string[] every registered type */ + public static function types(): array + { + self::registerDefaults(); + + return array_keys(self::$definitions); + } + + /** @return string[] the types whose tasks must run alone */ + public static function exclusiveTypes(): array + { + self::registerDefaults(); + + return array_keys(array_filter( + self::$definitions, + static fn (array $definition) => $definition['mode'] === self::MODE_EXCLUSIVE + )); + } + + /** + * The handler for a type, instantiated now. + * + * @throws InvalidArgumentException when the type is unknown or its handler + * does not satisfy the interface — both are programming errors in + * the registrant, not runtime conditions the worker should retry + */ + public static function handler(string $type): SystemTaskHandlerInterface + { + $definition = self::definition($type); + + if ($definition === null) { + throw new InvalidArgumentException('No handler is registered for system task type "' . $type . '".'); + } + + $handler = $definition['handler']; + + if (is_string($handler) && class_exists($handler)) { + $handler = new $handler(); + } elseif (!$handler instanceof SystemTaskHandlerInterface && is_callable($handler)) { + $handler = $handler(); + } + + if (!$handler instanceof SystemTaskHandlerInterface) { + throw new InvalidArgumentException( + 'The handler registered for system task type "' . $type . '" is not a ' + . SystemTaskHandlerInterface::class . '.' + ); + } + + return $handler; + } + + /** + * The callable that turns a manager request into a queued task, if the + * type declared one. A type without a creator can still be queued by the + * package's own code — it just is not reachable through the generic + * "create task from store request" entry point. + */ + public static function creator(string $type): ?callable + { + return self::definition($type)['creator'] ?? null; + } + + public static function mode(string $type): string + { + return (string) (self::definition($type)['mode'] ?? self::MODE_EXCLUSIVE); + } + + /** + * Whether a task of this type must run alone. + * + * An unknown type answers true. That is the safe direction: a task nothing + * claims to understand should not be assumed harmless to run alongside a + * site update. + */ + public static function isExclusive(string $type): bool + { + return self::mode($type) === self::MODE_EXCLUSIVE; + } + + public static function parallelism(string $type): int + { + return max(1, (int) (self::definition($type)['parallelism'] ?? 1)); + } + + /** @return string[] permissions required *in addition* to `exec_module` */ + public static function permissions(string $type): array + { + return (array) (self::definition($type)['permissions'] ?? []); + } + + public static function requiresSuperAdmin(string $type): bool + { + return (bool) (self::definition($type)['super_admin'] ?? false); + } + + public static function label(string $type): string + { + return (string) (self::definition($type)['label'] ?? $type); + } + + /** + * The three flows the CMS ships, declared rather than hardcoded. + * + * They keep the exact permissions and the exact super-admin gate the + * switch statements applied before, so an installation with no packages + * behaves identically. + */ + private static function registerDefaults(): void + { + if (self::$defaultsRegistered) { + return; + } + + self::$defaultsRegistered = true; + + self::$definitions['console_install'] = [ + 'type' => 'console_install', + 'handler' => ConsoleInstallFlowService::class, + 'mode' => self::MODE_EXCLUSIVE, + 'parallelism' => 1, + 'permissions' => ['system_tasks.manage_packages'], + 'super_admin' => false, + 'creator' => null, + 'label' => 'Console package install', + ]; + + self::$definitions['console_uninstall'] = [ + 'type' => 'console_uninstall', + 'handler' => ConsoleUninstallFlowService::class, + 'mode' => self::MODE_EXCLUSIVE, + 'parallelism' => 1, + 'permissions' => ['system_tasks.manage_packages'], + 'super_admin' => false, + 'creator' => null, + 'label' => 'Console package uninstall', + ]; + + self::$definitions['site_update'] = [ + 'type' => 'site_update', + 'handler' => SiteUpdateFlowService::class, + 'mode' => self::MODE_EXCLUSIVE, + 'parallelism' => 1, + 'permissions' => ['system_tasks.site_update'], + 'super_admin' => true, + 'creator' => null, + 'label' => 'Site update', + ]; + } +} diff --git a/core/src/Services/SystemTasks/SystemTaskService.php b/core/src/Services/SystemTasks/SystemTaskService.php index cc6cd3c8f8..4f87f321f2 100644 --- a/core/src/Services/SystemTasks/SystemTaskService.php +++ b/core/src/Services/SystemTasks/SystemTaskService.php @@ -10,6 +10,14 @@ class SystemTaskService { public const DEFAULT_LEASE_SECONDS = 900; + /** + * How far down the queue one acquire attempt looks for a startable task. + * + * Bounded so a queue holding thousands of blocked tasks cannot turn a + * once-a-minute worker tick into a full table scan. + */ + public const ACQUIRE_SCAN_LIMIT = 50; + protected CatalogService $catalogService; public function __construct(?CatalogService $catalogService = null) @@ -62,6 +70,32 @@ public function createTaskFromStoreRequest($type, array $request, array $request return $response; } + // Types a package registered. The built-in three keep the explicit + // cases above because each takes a differently shaped request; a + // registered type declares one creator and receives the request whole, + // so the store endpoint needs no knowledge of what is in it. + $creator = SystemTaskRegistry::creator($type); + + if ($creator !== null) { + $response = $creator($request, $requesterSnapshot, (bool) $isSuperAdmin); + + if (!is_array($response)) { + return [ + 'ok' => false, + 'error_code' => 'TASK_CREATE_FAILED', + 'message' => 'The creator registered for system task type "' . $type . '" returned no result.', + ]; + } + + if (!empty($response['ok'])) { + $response['warnings'] = $preflight['warnings']; + } + + return $response; + } + + // Registered, runnable, but with no creator: the package queues it + // through its own code and deliberately did not expose it here. return [ 'ok' => false, 'error_code' => 'TASK_TYPE_NOT_ALLOWED', @@ -349,48 +383,118 @@ public function cancelQueuedTaskPayload($id = 0, $uuid = '', array $requesterSna ]; } + /** + * Claim the next task this worker is allowed to start. + * + * Oldest first, but a candidate is skipped rather than the whole scan + * abandoned when its type may not start yet: an exclusive task waiting for + * a running one must not stop a concurrent task behind it from being + * picked, or one long install would idle the worker for its whole duration. + * + * The claim itself is an optimistic `UPDATE … WHERE status = 'queued'` and + * is the only thing that makes two workers safe on one queue — the + * concurrency check above it is advisory and can race, which is why the + * scan continues when the update touches no rows. + */ public function acquireNextQueuedTask($lockOwner, $host = '', $pid = null, $leaseSeconds = self::DEFAULT_LEASE_SECONDS) { - $candidate = SystemCliTask::query() + $candidates = SystemCliTask::query() ->where('status', 'queued') ->orderBy('id') - ->first(); + ->limit(self::ACQUIRE_SCAN_LIMIT) + ->get(); - if (!$candidate) { - return null; + foreach ($candidates as $candidate) { + if (!$this->canStartTaskNow($candidate)) { + continue; + } + + $now = Carbon::now(); + $updated = SystemCliTask::query() + ->where('id', $candidate->id) + ->where('status', 'queued') + ->update([ + 'status' => 'picked', + 'step' => 'picked', + 'progress' => 5, + 'message' => 'Picked by worker', + 'locked_by' => trim((string) $lockOwner), + 'attempt_count' => ((int) $candidate->attempt_count) + 1, + 'lease_expires_at' => $now->copy()->addSeconds((int) $leaseSeconds), + 'worker_host' => trim((string) $host), + 'worker_pid' => $pid, + 'started_at' => $candidate->started_at ?: $now, + 'heartbeat_at' => $now, + 'updated_at' => $now, + ]); + + if ((int) $updated !== 1) { + // Another worker won the race for this row. The next candidate + // may still be ours. + continue; + } + + $task = SystemCliTask::query()->find($candidate->id); + if ($task) { + $this->appendLog($task, 'info', 'picked', 'Task picked by worker.', [ + 'lock_owner' => trim((string) $lockOwner), + ]); + } + + return $task; } - $now = Carbon::now(); - $updated = SystemCliTask::query() - ->where('id', $candidate->id) - ->where('status', 'queued') - ->update([ - 'status' => 'picked', - 'step' => 'picked', - 'progress' => 5, - 'message' => 'Picked by worker', - 'locked_by' => trim((string) $lockOwner), - 'attempt_count' => ((int) $candidate->attempt_count) + 1, - 'lease_expires_at' => $now->copy()->addSeconds((int) $leaseSeconds), - 'worker_host' => trim((string) $host), - 'worker_pid' => $pid, - 'started_at' => $candidate->started_at ?: $now, - 'heartbeat_at' => $now, - 'updated_at' => $now, - ]); - - if ((int) $updated !== 1) { - return null; + return null; + } + + /** + * May a task of this type start while the queue looks like this? + * + * An unregistered type answers true on purpose. It cannot run, and letting + * the worker pick it is what turns it into a task marked failed with + * TASK_TYPE_NOT_ALLOWED — refusing to pick it here would instead leave it + * queued forever, in front of everything behind it. + */ + protected function canStartTaskNow(SystemCliTask $task): bool + { + $type = (string) $task->type; + + if (!SystemTaskRegistry::has($type)) { + return true; } - $task = SystemCliTask::query()->find($candidate->id); - if ($task) { - $this->appendLog($task, 'info', 'picked', 'Task picked by worker.', [ - 'lock_owner' => trim((string) $lockOwner), - ]); + if (SystemTaskRegistry::isExclusive($type)) { + return $this->countActiveTasks(SystemTaskRegistry::exclusiveTypes()) === 0; } - return $task; + return $this->countActiveTasks([$type]) < SystemTaskRegistry::parallelism($type); + } + + /** + * How many tasks of these types a worker is currently holding. + * + * Counts claimed work only — `picked` and `running` — and only while the + * lease is live, so an abandoned task stops occupying a slot once its + * lease lapses instead of shrinking the pipe permanently. + * + * @param string[] $types + */ + protected function countActiveTasks(array $types): int + { + if ($types === []) { + return 0; + } + + $now = Carbon::now(); + + return (int) SystemCliTask::query() + ->whereIn('status', ['picked', 'running']) + ->whereIn('type', $types) + ->where(function ($query) use ($now) { + $query->whereNull('lease_expires_at') + ->orWhere('lease_expires_at', '>=', $now); + }) + ->count(); } public function updateTaskProgress(SystemCliTask $task, $status, $progress, $step, $message, $level = null, array $context = []) @@ -840,39 +944,54 @@ protected function loadInstalledPackageComposer($composerName) protected function runCreatePreflight($type, array $requesterSnapshot = [], $isSuperAdmin = false) { - if (empty($requesterSnapshot['permissions']['exec_module'])) { + // An unknown type is refused before anything else is disclosed. It has + // no handler, so it could never run; answering it with a scheduler + // health verdict would only tell an unauthorised prober how the + // installation is doing. + if (!SystemTaskRegistry::has($type)) { return [ 'ok' => false, - 'error_code' => 'ACL_DENIED', - 'message' => 'System task creation requires module execution permission.', + 'error_code' => 'TASK_TYPE_NOT_ALLOWED', + 'message' => 'Unsupported system task type.', ]; } - if (in_array($type, ['console_install', 'console_uninstall'], true) && empty($requesterSnapshot['permissions']['system_tasks.manage_packages'])) { + if (empty($requesterSnapshot['permissions']['exec_module'])) { return [ 'ok' => false, 'error_code' => 'ACL_DENIED', - 'message' => 'Console package queueing requires system task package management permission.', + 'message' => 'System task creation requires module execution permission.', ]; } - if ($type === 'site_update' && empty($requesterSnapshot['permissions']['system_tasks.site_update'])) { - return [ - 'ok' => false, - 'error_code' => 'ACL_DENIED', - 'message' => 'Site update queueing requires system task site update permission.', - ]; + // Which extra permissions a type needs is the type's own declaration, + // so a package gates its work with its own permission key instead of + // borrowing one of the CMS's. + foreach (SystemTaskRegistry::permissions($type) as $permission) { + if (empty($requesterSnapshot['permissions'][$permission])) { + return [ + 'ok' => false, + 'error_code' => 'ACL_DENIED', + 'message' => 'Queueing a "' . SystemTaskRegistry::label($type) + . '" task requires the ' . $permission . ' permission.', + ]; + } } - if ($type === 'site_update' && !$isSuperAdmin) { + if (SystemTaskRegistry::requiresSuperAdmin($type) && !$isSuperAdmin) { return [ 'ok' => false, 'error_code' => 'ACL_DENIED', - 'message' => 'Site update tasks are limited to super administrators.', + 'message' => '"' . SystemTaskRegistry::label($type) . '" tasks are limited to super administrators.', ]; } - $activeTask = $this->findActiveMutatingTask(); + // The global lock covers exclusive types only, and is asked about only + // when the new task is itself exclusive. Before the registry every type + // was exclusive, so for the three built-in flows this is the behaviour + // it always had; what it stops doing is letting a package's long batch + // hold the updater hostage for the length of the batch. + $activeTask = SystemTaskRegistry::isExclusive($type) ? $this->findActiveMutatingTask() : null; if ($activeTask) { $activeTaskPayload = $this->buildTaskStatusPayload($activeTask); $activeTaskPayload['can_cancel_queued'] = ((string) $activeTask->status === 'queued'); @@ -897,12 +1016,16 @@ protected function runCreatePreflight($type, array $requesterSnapshot = [], $isS ]; } - if ($type === 'site_update') { + // A type reserved for super administrators is one that rewrites the + // installation, and half-healthy infrastructure is a bad place to start + // one: a lease that expires mid-update leaves a partly-updated site. + // Everything else is allowed to start and merely warned about. + if (SystemTaskRegistry::requiresSuperAdmin($type)) { if (($schedulerStatus['status'] ?? 'unhealthy') === 'degraded') { return [ 'ok' => false, 'error_code' => 'SITE_UPDATE_BLOCKED', - 'message' => 'Site update tasks are blocked while scheduler health is degraded.', + 'message' => SystemTaskRegistry::label($type) . ' tasks are blocked while scheduler health is degraded.', ]; } @@ -910,10 +1033,10 @@ protected function runCreatePreflight($type, array $requesterSnapshot = [], $isS return [ 'ok' => false, 'error_code' => 'SITE_UPDATE_BLOCKED', - 'message' => 'Site update tasks are blocked while worker health is unhealthy.', + 'message' => SystemTaskRegistry::label($type) . ' tasks are blocked while worker health is unhealthy.', ]; } - } elseif (in_array($type, ['console_install', 'console_uninstall'], true)) { + } else { if (($schedulerStatus['status'] ?? '') === 'degraded') { $warnings[] = [ 'code' => 'SCHEDULER_DEGRADED', @@ -935,10 +1058,39 @@ protected function runCreatePreflight($type, array $requesterSnapshot = [], $isS ]; } + /** + * The exclusive task currently holding the queue, if any. + * + * Only exclusive types count. A concurrent type is ordinary background + * work — a package's image batch may run for a day, and letting that block + * a site update for a day would make the shared queue unusable for exactly + * the work it was opened up for. + */ protected function findActiveMutatingTask() { + $exclusiveTypes = SystemTaskRegistry::exclusiveTypes(); + + if ($exclusiveTypes === []) { + return null; + } + + $now = Carbon::now(); + return SystemCliTask::query() ->whereIn('status', ['queued', 'picked', 'running']) + ->whereIn('type', $exclusiveTypes) + ->where(function ($query) use ($now) { + // A queued task holds the lock unconditionally — it has no + // lease yet. A picked or running one holds it only while its + // lease is live: a worker killed mid-task would otherwise hold + // the global lock forever, and every later task would be + // refused with GLOBAL_LOCK_ACTIVE until somebody edited the row + // by hand. A null lease is treated as live, because that is the + // shape a task takes between being claimed and being stamped. + $query->where('status', 'queued') + ->orWhereNull('lease_expires_at') + ->orWhere('lease_expires_at', '>=', $now); + }) ->orderBy('id') ->first(); } diff --git a/core/src/Support/ArithmeticExpression.php b/core/src/Support/ArithmeticExpression.php new file mode 100644 index 0000000000..5c869ecef3 --- /dev/null +++ b/core/src/Support/ArithmeticExpression.php @@ -0,0 +1,466 @@ +', + '==', '!=', '<>', '<=', '>=', '&&', '||', + '+', '-', '*', '/', '%', '<', '>', '!', + ]; + + /** + * Binary operator precedence, mirroring PHP's own table. Higher binds tighter. + */ + private const PRECEDENCE = [ + '*' => 60, '/' => 60, '%' => 60, + '+' => 50, '-' => 50, + '<' => 40, '<=' => 40, '>' => 40, '>=' => 40, '<=>' => 40, + '==' => 30, '!=' => 30, '<>' => 30, '===' => 30, '!==' => 30, + '&&' => 20, + '||' => 10, + ]; + + /** + * Unary operators, all right-associative and binding tighter than any binary operator. + */ + private const UNARY = ['u+' => 70, 'u-' => 70, '!' => 70]; + + /** + * Bounds that keep a hostile expression from costing more than it is worth. Real modifier + * arguments are a handful of characters; these are orders of magnitude above anything genuine. + */ + private const MAX_LENGTH = 512; + private const MAX_TOKENS = 256; + private const MAX_DEPTH = 32; + + /** + * Evaluate an expression, falling back to $default when it is not one we accept. + * + * @param mixed $expression + * @param mixed $default + * @return mixed + */ + public static function evaluate($expression, $default = 0) + { + $result = self::tryEvaluate($expression); + + return $result === null ? $default : $result; + } + + /** + * Evaluate an expression, returning null when it is not one we accept. + * + * @param mixed $expression + * @return int|float|bool|null + */ + public static function tryEvaluate($expression) + { + if (!is_scalar($expression)) { + return null; + } + + $expression = trim((string) $expression); + if ($expression === '' || strlen($expression) > self::MAX_LENGTH) { + return null; + } + + $tokens = self::tokenize($expression); + if ($tokens === null) { + return null; + } + + $rpn = self::toReversePolish($tokens); + if ($rpn === null) { + return null; + } + + return self::evaluateReversePolish($rpn); + } + + /** + * Split the expression into number literals, operators and parentheses. + * + * Unary `+`/`-`/`!` are distinguished from their binary forms here, while we still know whether + * the previous token closed an operand. + * + * @param string $expression + * @return array|null + */ + private static function tokenize($expression) + { + $tokens = []; + $length = strlen($expression); + $offset = 0; + // False directly after an operand (a number or a closing paren), which is the only position + // where `+`/`-` are binary. + $expectOperand = true; + + while ($offset < $length) { + if (count($tokens) > self::MAX_TOKENS) { + return null; + } + + $char = $expression[$offset]; + + if ($char === ' ' || $char === "\t" || $char === "\n" || $char === "\r") { + $offset++; + continue; + } + + if ($char === '(') { + if (!$expectOperand) { + // `2(3)` was never valid PHP either. + return null; + } + $tokens[] = ['(', null]; + $offset++; + continue; + } + + if ($char === ')') { + if ($expectOperand) { + return null; + } + $tokens[] = [')', null]; + $offset++; + $expectOperand = false; + continue; + } + + if (self::isDigit($char) || ($char === '.' && isset($expression[$offset + 1]) && self::isDigit($expression[$offset + 1]))) { + if (!$expectOperand) { + return null; + } + $number = self::readNumber($expression, $offset); + if ($number === null) { + return null; + } + $tokens[] = ['num', $number]; + $expectOperand = false; + continue; + } + + $operator = self::readOperator($expression, $offset); + if ($operator === null) { + return null; + } + + if ($expectOperand) { + // Only `+`, `-` and `!` have a unary form; anything else here is a syntax error. + if ($operator === '+' || $operator === '-') { + $tokens[] = ['op', 'u' . $operator]; + continue; + } + if ($operator === '!') { + $tokens[] = ['op', '!']; + continue; + } + + return null; + } + + if (!isset(self::PRECEDENCE[$operator])) { + // `!` cannot be binary. + return null; + } + + $tokens[] = ['op', $operator]; + $expectOperand = true; + } + + if ($expectOperand || $tokens === []) { + // A trailing operator, or nothing at all. + return null; + } + + return $tokens; + } + + /** + * @param string $char + * @return bool + */ + private static function isDigit($char) + { + return $char >= '0' && $char <= '9'; + } + + /** + * Read one decimal literal, advancing $offset past it. + * + * Exponents are deliberately unsupported: the callers strip `e` before we ever see the string, + * so accepting them here would only invent a syntax that never worked. + * + * @param string $expression + * @param int $offset + * @return int|float|null + */ + private static function readNumber($expression, &$offset) + { + $start = $offset; + $length = strlen($expression); + $seenDot = false; + + while ($offset < $length) { + $char = $expression[$offset]; + if (self::isDigit($char)) { + $offset++; + continue; + } + if ($char === '.' && !$seenDot) { + $seenDot = true; + $offset++; + continue; + } + break; + } + + $literal = substr($expression, $start, $offset - $start); + if ($literal === '' || $literal === '.') { + return null; + } + + if (!$seenDot && ctype_digit($literal)) { + // Stay on int while the value fits, so `2*3` keeps returning int(6) as eval() did. + $asInt = (int) $literal; + if ((string) $asInt === ltrim($literal, '0') || $literal === '0' || ltrim($literal, '0') === '') { + return $asInt; + } + + return (float) $literal; + } + + return (float) $literal; + } + + /** + * Read one operator, advancing $offset past it. + * + * @param string $expression + * @param int $offset + * @return string|null + */ + private static function readOperator($expression, &$offset) + { + foreach (self::OPERATORS as $operator) { + if (substr($expression, $offset, strlen($operator)) === $operator) { + $offset += strlen($operator); + + return $operator; + } + } + + return null; + } + + /** + * Shunting-yard: infix tokens to reverse polish notation. + * + * @param array $tokens + * @return array|null + */ + private static function toReversePolish(array $tokens) + { + $output = []; + $stack = []; + + foreach ($tokens as $token) { + [$type, $value] = $token; + + if ($type === 'num') { + $output[] = $token; + continue; + } + + if ($type === '(') { + $stack[] = $token; + if (count($stack) > self::MAX_DEPTH) { + return null; + } + continue; + } + + if ($type === ')') { + $matched = false; + while ($stack !== []) { + $top = array_pop($stack); + if ($top[0] === '(') { + $matched = true; + break; + } + $output[] = $top; + } + if (!$matched) { + return null; + } + continue; + } + + $isUnary = isset(self::UNARY[$value]); + $precedence = $isUnary ? self::UNARY[$value] : self::PRECEDENCE[$value]; + + while ($stack !== []) { + $top = end($stack); + if ($top[0] !== 'op') { + break; + } + $topIsUnary = isset(self::UNARY[$top[1]]); + $topPrecedence = $topIsUnary ? self::UNARY[$top[1]] : self::PRECEDENCE[$top[1]]; + + // Unary operators are right-associative, so an equal precedence does not pop. + if ($topPrecedence > $precedence || ($topPrecedence === $precedence && !$isUnary)) { + $output[] = array_pop($stack); + continue; + } + break; + } + + $stack[] = $token; + if (count($stack) > self::MAX_DEPTH) { + return null; + } + } + + while ($stack !== []) { + $top = array_pop($stack); + if ($top[0] === '(') { + return null; + } + $output[] = $top; + } + + return $output; + } + + /** + * @param array $rpn + * @return int|float|bool|null + */ + private static function evaluateReversePolish(array $rpn) + { + $stack = []; + + foreach ($rpn as $token) { + [$type, $value] = $token; + + if ($type === 'num') { + $stack[] = $value; + continue; + } + + if (isset(self::UNARY[$value])) { + if ($stack === []) { + return null; + } + $operand = array_pop($stack); + switch ($value) { + case 'u-': + $stack[] = -$operand; + break; + case 'u+': + $stack[] = +$operand; + break; + default: + $stack[] = !$operand; + } + continue; + } + + if (count($stack) < 2) { + return null; + } + $right = array_pop($stack); + $left = array_pop($stack); + + $result = self::apply($value, $left, $right); + if ($result === null) { + return null; + } + $stack[] = $result; + } + + if (count($stack) !== 1) { + return null; + } + + return $stack[0]; + } + + /** + * Apply one binary operator using PHP's own semantics. + * + * @param string $operator + * @param int|float|bool $left + * @param int|float|bool $right + * @return int|float|bool|null + */ + private static function apply($operator, $left, $right) + { + switch ($operator) { + case '+': + return $left + $right; + case '-': + return $left - $right; + case '*': + return $left * $right; + case '/': + // eval() raised DivisionByZeroError here; reporting "not evaluable" lets the caller + // fall back to its default instead of taking the request down. + if ((float) $right === 0.0) { + return null; + } + + return $left / $right; + case '%': + if ((int) $right === 0) { + return null; + } + + return (int) $left % (int) $right; + case '<': + return $left < $right; + case '<=': + return $left <= $right; + case '>': + return $left > $right; + case '>=': + return $left >= $right; + case '<=>': + return $left <=> $right; + case '==': + return $left == $right; + case '===': + return $left === $right; + case '!=': + case '<>': + return $left != $right; + case '!==': + return $left !== $right; + case '&&': + return (bool) $left && (bool) $right; + case '||': + return (bool) $left || (bool) $right; + } + + return null; + } +} diff --git a/core/tests/Unit/Console/SiteUpdateCommandTest.php b/core/tests/Unit/Console/SiteUpdateCommandTest.php index 6dddb785bd..ed356939e5 100644 --- a/core/tests/Unit/Console/SiteUpdateCommandTest.php +++ b/core/tests/Unit/Console/SiteUpdateCommandTest.php @@ -215,3 +215,50 @@ function invokeSiteUpdateMethod(SiteUpdateCommand $command, string $method, arra expect($params['guid'])->toBe('store435243542tf542t5t'); expect($moduleCode)->toContain("assets/modules/store/core.php"); }); + +test('composer detection uses a probe the local shell actually understands', function () { + $source = file_get_contents(dirname(__DIR__, 3) . '/src/Console/SiteUpdateCommand.php'); + + expect($source) + ->toContain("'where ' . escapeshellarg(\$command) . ' >NUL 2>NUL'") + ->toContain("'command -v ' . escapeshellarg(\$command) . ' >/dev/null 2>&1'"); +}); + +test('shellCommandExists finds a command that is really on PATH', function () { + $present = windows_os() ? 'where' : 'sh'; + + expect(invokeSiteUpdateMethod($this->command, 'shellCommandExists', [$present]))->toBeTrue() + ->and(invokeSiteUpdateMethod($this->command, 'shellCommandExists', ['evo-no-such-command-here']))->toBeFalse(); +}); + +test('composer binary candidates cover windows layouts as well as posix ones', function () { + $programData = getenv('ProgramData'); + $appData = getenv('APPDATA'); + + putenv('ProgramData=C:\ProgramData'); + putenv('APPDATA=C:\Users\evo\AppData\Roaming'); + + $candidates = invokeSiteUpdateMethod($this->command, 'composerBinaryCandidates'); + + expect($candidates) + ->toContain('/usr/local/bin/composer') + ->toContain('C:/ProgramData/ComposerSetup/bin/composer.bat') + ->toContain('C:/Users/evo/AppData/Roaming/Composer/composer.bat'); + + $programData === false ? putenv('ProgramData') : putenv('ProgramData=' . $programData); + $appData === false ? putenv('APPDATA') : putenv('APPDATA=' . $appData); +}); + +test('isExecutableFile accepts a windows shim that is_executable rejects', function () { + $path = sys_get_temp_dir() . '/evo-composer-probe-' . getmypid() . '.bat'; + file_put_contents($path, '@echo off'); + + try { + $expected = windows_os() ? true : is_executable($path); + + expect(invokeSiteUpdateMethod($this->command, 'isExecutableFile', [$path]))->toBe($expected) + ->and(invokeSiteUpdateMethod($this->command, 'isExecutableFile', [$path . '.missing']))->toBeFalse(); + } finally { + @unlink($path); + } +}); diff --git a/core/tests/Unit/DatabaseBackupServiceConsistencyTest.php b/core/tests/Unit/DatabaseBackupServiceConsistencyTest.php index 27b508af1d..ad7b980bc7 100644 --- a/core/tests/Unit/DatabaseBackupServiceConsistencyTest.php +++ b/core/tests/Unit/DatabaseBackupServiceConsistencyTest.php @@ -20,8 +20,36 @@ expect($source) ->toContain('buildTempSnapshotFilePath') - ->toContain('>> \' . escapeshellarg((string) $tempFilePath)') + ->toContain('fopen($tempFilePath, \'ab\')') ->toContain('unlink($tempFilePath)') ->toContain('rename($tempFilePath, (string) $filePath)') - ->not->toContain('>> \' . escapeshellarg((string) $filePath)'); + // The dump must never be written straight to the published path, or a + // failed run would leave a truncated snapshot where a good one was. + ->not->toContain('fopen((string) $filePath'); +}); + +test('postgres snapshots pass the password by environment, not a shell prefix', function () { + $source = file_get_contents(__DIR__ . '/../../src/Services/DatabaseBackupService.php'); + + expect($source) + // A leading `PGPASSWORD=… pg_dump` assignment is POSIX shell syntax. + // cmd.exe answers "'PGPASSWORD' is not recognized", so that form could + // never produce a backup on Windows. Matched on the concatenation the + // old code used rather than on the bare name, which still appears in + // the comment explaining why it went away. + ->not->toContain("'PGPASSWORD=' . escapeshellarg") + ->toContain('[\'PGPASSWORD\' => $password]') + ->toContain('new Process('); +}); + +test('postgres dump output is streamed rather than buffered in memory', function () { + $source = file_get_contents(__DIR__ . '/../../src/Services/DatabaseBackupService.php'); + + // A dump is as large as the database; collecting it with getOutput() + // before writing would blow memory on exactly the databases worth backing + // up. The run callback writes each chunk straight to the handle. + expect($source) + ->toContain('$process->run(static function ($type, $buffer) use ($handle)') + ->toContain('fwrite($handle, $buffer)') + ->not->toContain('fwrite($handle, $process->getOutput())'); }); diff --git a/core/tests/Unit/Security/ParserEvalHardeningTest.php b/core/tests/Unit/Security/ParserEvalHardeningTest.php new file mode 100644 index 0000000000..1475ab835d --- /dev/null +++ b/core/tests/Unit/Security/ParserEvalHardeningTest.php @@ -0,0 +1,236 @@ + conditional tags +| - _getSGVar() [[$_GET(x)]] superglobal reads +| - atBindFileContent() @FILE: template includes +| +| All three are reachable from content that the parser re-scans across passes, so a snippet echoing +| request data can carry a payload into them without any editing privilege. These tests drive the +| real methods on a Core instance and assert that a payload cannot execute or read outside the tree, +| while the legitimate syntax each method exists to serve keeps working. +| +*/ + +use EvolutionCMS\Core; + +beforeAll(function () { + if (!defined('IN_INSTALL_MODE')) { + define('IN_INSTALL_MODE', false); + } + if (!defined('EVO_API_MODE')) { + define('EVO_API_MODE', true); + } + if (!defined('IN_MANAGER_MODE')) { + define('IN_MANAGER_MODE', false); + } + $root = str_replace('\\', '/', dirname(__DIR__, 3)) . '/'; + if (!defined('EVO_BASE_PATH')) { + define('EVO_BASE_PATH', $root); + } + if (!defined('EVO_CORE_PATH')) { + define('EVO_CORE_PATH', $root . 'core/'); + } + if (!defined('EVO_MANAGER_PATH')) { + define('EVO_MANAGER_PATH', $root . 'manager/'); + } + $autoload = EVO_CORE_PATH . 'vendor/autoload.php'; + if (file_exists($autoload)) { + require_once $autoload; + } +}); + +class ParserHardeningCore extends Core +{ + public $cfg = ['enable_filter' => 1, 'rb_base_url' => 'assets/']; + + public function getConfig($name = '', $default = null) + { + return $this->cfg[$name] ?? $default; + } + + public function setConfig($name, $value = null): void + { + $this->cfg[$name] = $value; + } +} + +/** + * A Core with the two config keys the tested methods read, built without the heavy constructor so + * no bootstrap (storage paths, container, DB) is required. + */ +function parserHardeningCore(): Core +{ + $core = (new ReflectionClass(ParserHardeningCore::class))->newInstanceWithoutConstructor(); + + $_SERVER['REQUEST_TIME'] = $_SERVER['REQUEST_TIME'] ?? time(); + + return $core; +} + +describe('conditional tags (<@IF:>)', function () { + + test('a quote/backslash breakout neither executes nor fatals', function () { + $core = parserHardeningCore(); + $marker = str_replace('\\', '/', sys_get_temp_dir()) . '/evo_ctag_' . bin2hex(random_bytes(6)); + + // A backslash immediately before the quote defeated the str_replace("'", "\'") escaping: + // the doubled backslash was consumed, the quote closed the generated string literal early, + // and the tail ran as PHP. + $bs = chr(92); + $sq = chr(39); + $cmd = '1' . $bs . $sq . '.file_put_contents("' . $marker . '","x").' . $bs . $sq . '1'; + + $out = $core->mergeConditionalTagsContent('<@IF:' . $cmd . '>body<@ENDIF>'); + + expect(file_exists($marker))->toBeFalse() + ->and($out)->toBeString(); + }); + + test('legitimate numeric conditionals still resolve', function () { + $core = parserHardeningCore(); + + expect($core->mergeConditionalTagsContent('<@IF:5>A<@ELSE>B<@ENDIF>'))->toBe('A') + ->and($core->mergeConditionalTagsContent('<@IF:0>A<@ELSE>B<@ENDIF>'))->toBe('B') + ->and($core->mergeConditionalTagsContent('<@IF:5>A<@ELSEIF:1>X<@ELSE>B<@ENDIF>'))->toBe('A') + ->and($core->mergeConditionalTagsContent('<@IF:0>A<@ELSEIF:1>X<@ELSE>B<@ENDIF>'))->toBe('X') + ->and($core->mergeConditionalTagsContent('<@IF: !0 >neg<@ENDIF>'))->toBe('neg'); + }); + + test('nested conditionals resolve without index corruption', function () { + $core = parserHardeningCore(); + + // The inner block trims the shared command list; the outer indices must survive that. + $tpl = '<@IF:1>outer <@IF:1>inner<@ELSE>x<@ENDIF> end<@ELSE>no<@ENDIF>'; + + expect($core->mergeConditionalTagsContent($tpl))->toBe('outer inner end'); + }); + + test('content with no conditional tag is returned untouched', function () { + $core = parserHardeningCore(); + $plain = 'plain [+ph+] content with no tags'; + + expect($core->mergeConditionalTagsContent($plain))->toBe($plain); + }); +}); + +describe('superglobal reads ([[$_GET(x)]])', function () { + + test('a backtick payload is not executed', function () { + $core = parserHardeningCore(); + // A colon-free relative name: a `:` in the tag is the modifier delimiter, unrelated here. + $marker = 'evo_sg_' . bin2hex(random_bytes(6)) . '.txt'; + + // Backticks need no parentheses, so the old `(`/`)` rewrite did not stop them. + $payload = '$_SERVER . `echo x > ' . $marker . '`'; + + $value = $core->_getSGVar($payload); + + expect(file_exists($marker))->toBeFalse() + ->and($value)->toBe(''); + }); + + test('a statement-separator payload is refused', function () { + $core = parserHardeningCore(); + + expect($core->_getSGVar('$_GET[id];phpinfo()'))->toBe(''); + }); + + test('the documented accessor forms still read the value', function () { + $core = parserHardeningCore(); + $_GET['id'] = 'hello'; + $_POST['name'] = 'world'; + + // The caller rewrites (key) into ['key'] before _getSGVar sees it; accept both spellings. + expect($core->_getSGVar("\$_GET['id']"))->toBe('hello') + ->and($core->_getSGVar('$_GET(id)'))->toBe('hello') + ->and($core->_getSGVar("\$_POST['name']"))->toBe('world'); + + unset($_GET['id'], $_POST['name']); + }); + + test('a missing key yields empty string, not a notice', function () { + $core = parserHardeningCore(); + unset($_GET['nope']); + + expect($core->_getSGVar("\$_GET['nope']"))->toBe(''); + }); + + test('mgrFormValues and token stay hidden from $_SESSION dumps', function () { + $core = parserHardeningCore(); + $_SESSION = ['visible' => '1', 'mgrFormValues' => 'secret', 'token' => 'csrf']; + + $dump = $core->_getSGVar('$_SESSION'); + + expect($dump)->toContain('visible') + ->and($dump)->not->toContain('mgrFormValues') + ->and($dump)->not->toContain('csrf'); + + $_SESSION = []; + }); + + test('a variable outside the allow list is refused', function () { + $core = parserHardeningCore(); + + expect($core->_getSGVar('$GLOBALS'))->toBe('') + ->and($core->_getSGVar('$this'))->toBe(''); + }); +}); + +describe('@FILE binding', function () { + + test('directory traversal outside the base path is refused', function () { + $core = parserHardeningCore(); + + // A real file that certainly exists outside EVO_BASE_PATH. + $traversalDepth = str_repeat('../', 20); + + expect($core->atBindFileContent('@FILE:' . $traversalDepth . 'Windows/win.ini')) + ->toContain('Could not retrieve') + ->and($core->atBindFileContent('@FILE:' . $traversalDepth . 'etc/passwd')) + ->toContain('Could not retrieve'); + }); + + test('a php file inside the tree is still refused, including alternate extensions', function () { + $core = parserHardeningCore(); + + expect($core->atBindFileContent('@FILE:index.php'))->toBe('Could not retrieve PHP file.') + ->and($core->atBindFileContent('@FILE:index.phtml'))->toBe('Could not retrieve PHP file.') + ->and($core->atBindFileContent('@FILE:x.inc'))->toBe('Could not retrieve PHP file.'); + }); + + test('a permitted file inside the tree is read', function () { + $core = parserHardeningCore(); + + $relative = 'assets/evo_atfile_' . bin2hex(random_bytes(6)) . '.txt'; + $absolute = EVO_BASE_PATH . $relative; + file_put_contents($absolute, 'included-body'); + + try { + expect($core->atBindFileContent('@FILE:' . $relative))->toBe('included-body'); + } finally { + @unlink($absolute); + } + }); + + test('a traversal that resolves back inside the tree is still allowed', function () { + $core = parserHardeningCore(); + + $relative = 'assets/evo_atfile_' . bin2hex(random_bytes(6)) . '.txt'; + $absolute = EVO_BASE_PATH . $relative; + file_put_contents($absolute, 'roundtrip'); + + try { + // assets/../assets/ normalises to a path under the base, so it must resolve. + expect($core->atBindFileContent('@FILE:assets/../' . $relative))->toBe('roundtrip'); + } finally { + @unlink($absolute); + } + }); +}); diff --git a/core/tests/Unit/Support/ArithmeticExpressionTest.php b/core/tests/Unit/Support/ArithmeticExpressionTest.php new file mode 100644 index 0000000000..9eda282bed --- /dev/null +++ b/core/tests/Unit/Support/ArithmeticExpressionTest.php @@ -0,0 +1,160 @@ +toBe($expected); + })->with([ + '1+1', + '2*3', + '10-4', + '7/2', + '6/3', + '10%3', + '2+3*4', + '(2+3)*4', + '((1+2)*(3+4))', + '-5+3', + '+5-3', + '2*-3', + '1.5+2.25', + '0.1*3', + '100/8', + '1<2', + '2<=2', + '3>4', + '4>=4', + '1==1', + '1!=2', + '1&&0', + '1||0', + '!0', + '!1', + '2+3>4', + '1&&1||0', + '10-2-3', + '100/10/2', + '2*3%4', + '0', + '42', + '-0', + ]); +}); + +describe('operator handling', function () { + + test('left associativity is preserved for subtraction and division', function () { + expect(ArithmeticExpression::evaluate('10-2-3'))->toBe(5) + ->and(ArithmeticExpression::evaluate('100/10/2'))->toBe(5); + }); + + test('unary minus binds tighter than multiplication but not than parentheses', function () { + expect(ArithmeticExpression::evaluate('-2*3'))->toBe(-6) + ->and(ArithmeticExpression::evaluate('-(2*3)'))->toBe(-6) + ->and(ArithmeticExpression::evaluate('2--3'))->toBe(5); + }); + + test('integer arithmetic stays integer', function () { + expect(ArithmeticExpression::evaluate('2*3'))->toBeInt() + ->and(ArithmeticExpression::evaluate('6/3'))->toBeInt() + ->and(ArithmeticExpression::evaluate('7/2'))->toBeFloat(); + }); + + test('division and modulo by zero fall back instead of raising', function () { + // eval() raised DivisionByZeroError here, which took the whole request down. + expect(ArithmeticExpression::evaluate('1/0'))->toBe(0) + ->and(ArithmeticExpression::evaluate('1%0'))->toBe(0) + ->and(ArithmeticExpression::evaluate('1/0', 'n/a'))->toBe('n/a'); + }); +}); + +describe('rejects everything that is not arithmetic', function () { + + // Every payload below survives `preg_replace('@([a-zA-Z\n\r\t\s])@', '', $filter)` - the filter + // the callers apply before handing the string over - because it contains no letters at all. + $payloads = [ + 'octal escaped system() call' => '"\163\171\163\164\145\155"("\151\144")', + 'octal escaped phpinfo' => '"\160\150\160\151\156\146\157"()', + 'backtick shell operator' => '1 . `\151\144`', + 'variable variable' => '${"\137\107\105\124"}', + 'statement separator' => '1;print_r($_SERVER)', + 'superglobal read' => '$_SERVER', + 'string concatenation' => '"1"."2"', + 'array literal' => '[1,2][0]', + 'xor built string' => '("\1"^"\1")', + 'heredoc-ish quoting' => '"1"', + 'bare quote' => "'", + 'backslash' => '\\', + 'dollar' => '$', + 'braces' => '{1}', + 'closing paren only' => ')', + 'opening paren only' => '(', + 'unbalanced parens' => '(1+2', + 'trailing operator' => '1+', + 'leading binary operator' => '*2', + 'empty' => '', + 'two numbers' => '1 2', + 'implicit multiplication' => '2(3)', + ]; + + test('refuses the payload', function (string $payload) { + expect(ArithmeticExpression::tryEvaluate($payload))->toBeNull() + ->and(ArithmeticExpression::evaluate($payload))->toBe(0); + })->with($payloads); + + test('no payload reaches PHP even when it would be valid PHP', function () { + // If any of these were still evaluated the marker file would exist afterwards. + $marker = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'evo_arith_' . bin2hex(random_bytes(6)); + + // file_put_contents("", "x") spelled without a single letter. + $call = '"\146\151\154\145\137\160\165\164\137\143\157\156\164\145\156\164\163"("' + . addcslashes($marker, "\\\"") + . '","\170")'; + + ArithmeticExpression::evaluate($call); + + expect(file_exists($marker))->toBeFalse(); + }); +}); + +describe('bounds', function () { + + test('an over-long expression is refused rather than parsed', function () { + $long = str_repeat('1+', 400) . '1'; + + expect(ArithmeticExpression::tryEvaluate($long))->toBeNull(); + }); + + test('deeply nested parentheses are refused rather than recursed', function () { + $nested = str_repeat('(', 100) . '1' . str_repeat(')', 100); + + expect(ArithmeticExpression::tryEvaluate($nested))->toBeNull(); + }); + + test('a nesting depth a template might really use still works', function () { + expect(ArithmeticExpression::evaluate('((((1+2))))'))->toBe(3); + }); + + test('non-scalar input is refused', function () { + expect(ArithmeticExpression::tryEvaluate([1, 2]))->toBeNull() + ->and(ArithmeticExpression::tryEvaluate(null))->toBeNull(); + }); +}); diff --git a/core/tests/Unit/SystemTasks/SystemTaskRegistryTest.php b/core/tests/Unit/SystemTasks/SystemTaskRegistryTest.php new file mode 100644 index 0000000000..c46373c4a1 --- /dev/null +++ b/core/tests/Unit/SystemTasks/SystemTaskRegistryTest.php @@ -0,0 +1,408 @@ + 'Registry test handler finished.', 'result' => ['ok' => true]]; + } +} + +beforeAll(function () { + $capsule = new Capsule(); + $capsule->addConnection([ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + ]); + $capsule->setAsGlobal(); + $capsule->bootEloquent(); + Model::setConnectionResolver($capsule->getDatabaseManager()); + + $schema = $capsule->schema(); + + $schema->create('system_cli_tasks', function (Blueprint $table) { + $table->increments('id'); + $table->string('uuid', 36)->unique(); + $table->string('type', 64)->default(''); + $table->string('target', 191)->default(''); + $table->string('requested_version', 191)->default(''); + $table->string('status', 32)->default('queued'); + $table->string('step', 64)->default(''); + $table->unsignedSmallInteger('progress')->default(0); + $table->string('message', 255)->default(''); + $table->text('payload_json')->nullable(); + $table->text('result_json')->nullable(); + $table->unsignedInteger('created_by')->nullable(); + $table->string('locked_by', 191)->default(''); + $table->unsignedInteger('attempt_count')->default(0); + $table->dateTime('lease_expires_at')->nullable(); + $table->string('worker_host', 191)->default(''); + $table->integer('worker_pid')->nullable(); + $table->string('error_code', 64)->default(''); + $table->string('catalog_snapshot_hash', 64)->default(''); + $table->text('requested_by_snapshot')->nullable(); + $table->dateTime('started_at')->nullable(); + $table->dateTime('heartbeat_at')->nullable(); + $table->dateTime('cancellation_requested_at')->nullable(); + $table->dateTime('finished_at')->nullable(); + $table->dateTime('created_at')->nullable(); + $table->dateTime('updated_at')->nullable(); + }); + + $schema->create('system_cli_task_logs', function (Blueprint $table) { + $table->increments('id'); + $table->unsignedInteger('task_id'); + $table->unsignedInteger('seq')->default(0); + $table->string('level', 16)->default('info'); + $table->string('step', 64)->default(''); + $table->text('message'); + $table->text('context_json')->nullable(); + $table->dateTime('created_at')->nullable(); + }); + + $schema->create('system_scheduler_health', function (Blueprint $table) { + $table->unsignedTinyInteger('id')->primary(); + $table->dateTime('last_heartbeat_at')->nullable(); + $table->string('last_heartbeat_host', 191)->default(''); + $table->string('last_heartbeat_mode', 32)->default(''); + $table->dateTime('updated_at')->nullable(); + }); + + $schema->create('system_worker_health', function (Blueprint $table) { + $table->unsignedTinyInteger('id')->primary(); + $table->dateTime('last_worker_run_at')->nullable(); + $table->dateTime('last_worker_pick_at')->nullable(); + $table->dateTime('last_worker_success_at')->nullable(); + $table->dateTime('last_worker_failed_at')->nullable(); + $table->string('last_worker_error_code', 64)->default(''); + $table->string('last_worker_host', 191)->default(''); + $table->integer('last_worker_pid')->nullable(); + $table->dateTime('updated_at')->nullable(); + }); +}); + +/** The types these tests register. The registry is process-wide, so they are cleaned up around every test. */ +function registryTestTypes(): array +{ + return ['pkg.batch', 'pkg.exclusive', 'pkg.instance', 'pkg.factory', 'pkg.bogus', 'pkg.temp']; +} + +function forgetRegistryTestTypes(): void +{ + foreach (registryTestTypes() as $type) { + SystemTaskRegistry::forget($type); + } +} + +beforeEach(function () { + SystemCliTask::query()->delete(); + \EvolutionCMS\Models\SystemCliTaskLog::query()->delete(); + \EvolutionCMS\Models\SystemSchedulerHealth::query()->delete(); + \EvolutionCMS\Models\SystemWorkerHealth::query()->delete(); + forgetRegistryTestTypes(); + + // A concurrent type with room for three, used by most of the queue tests. + SystemTaskRegistry::register('pkg.batch', RegistryTestHandler::class, [ + 'mode' => SystemTaskRegistry::MODE_CONCURRENT, + 'parallelism' => 3, + 'permissions' => ['pkg.run'], + 'label' => 'Package batch', + ]); +}); + +afterEach(function () { + forgetRegistryTestTypes(); +}); + +function makeRegistryTask(string $type, string $status, ?Carbon $lease = null): SystemCliTask +{ + static $counter = 0; + $counter++; + + return SystemCliTask::query()->create([ + 'uuid' => 'registry-task-' . $counter, + 'type' => $type, + 'target' => '', + 'requested_version' => '', + 'status' => $status, + 'step' => $status, + 'progress' => 0, + 'message' => '', + 'payload_json' => [], + 'result_json' => [], + 'created_by' => 1, + 'locked_by' => '', + 'attempt_count' => 0, + 'lease_expires_at' => $lease, + 'worker_host' => '', + 'worker_pid' => null, + 'error_code' => '', + 'catalog_snapshot_hash' => '', + 'requested_by_snapshot' => ['user_id' => 1], + 'created_at' => Carbon::now(), + 'updated_at' => Carbon::now(), + ]); +} + +function runRegistryPreflight(SystemTaskService $service, string $type, array $snapshot, bool $isSuperAdmin = false): array +{ + $method = new ReflectionMethod($service, 'runCreatePreflight'); + $method->setAccessible(true); + + return $method->invoke($service, $type, $snapshot, $isSuperAdmin); +} + +function registryAdminSnapshot(): array +{ + return [ + 'user_id' => 7, + 'permissions' => [ + 'exec_module' => true, + 'system_tasks.view' => 1, + 'system_tasks.manage_packages' => 1, + 'system_tasks.site_update' => 1, + 'pkg.run' => 1, + ], + ]; +} + +// --------------------------------------------------------------------------- +// The built-in declarations must reproduce the switch statements they replaced +// --------------------------------------------------------------------------- + +test('the three built-in task types stay registered and exclusive', function () { + expect(SystemTaskRegistry::has('console_install'))->toBeTrue() + ->and(SystemTaskRegistry::has('console_uninstall'))->toBeTrue() + ->and(SystemTaskRegistry::has('site_update'))->toBeTrue() + ->and(SystemTaskRegistry::isExclusive('console_install'))->toBeTrue() + ->and(SystemTaskRegistry::isExclusive('console_uninstall'))->toBeTrue() + ->and(SystemTaskRegistry::isExclusive('site_update'))->toBeTrue(); +}); + +test('built-in permissions match the checks the registry replaced', function () { + expect(SystemTaskRegistry::permissions('console_install'))->toBe(['system_tasks.manage_packages']) + ->and(SystemTaskRegistry::permissions('console_uninstall'))->toBe(['system_tasks.manage_packages']) + ->and(SystemTaskRegistry::permissions('site_update'))->toBe(['system_tasks.site_update']) + ->and(SystemTaskRegistry::requiresSuperAdmin('site_update'))->toBeTrue() + ->and(SystemTaskRegistry::requiresSuperAdmin('console_install'))->toBeFalse(); +}); + +test('an unknown type is unregistered and assumed exclusive', function () { + expect(SystemTaskRegistry::has('nope'))->toBeFalse() + ->and(SystemTaskRegistry::isExclusive('nope'))->toBeTrue(); +}); + +// --------------------------------------------------------------------------- +// Registration +// --------------------------------------------------------------------------- + +test('a malformed task type is refused', function () { + expect(fn () => SystemTaskRegistry::register('', RegistryTestHandler::class)) + ->toThrow(InvalidArgumentException::class); + + expect(fn () => SystemTaskRegistry::register(str_repeat('a', 65), RegistryTestHandler::class)) + ->toThrow(InvalidArgumentException::class); + + expect(fn () => SystemTaskRegistry::register('Bad_Type', RegistryTestHandler::class)) + ->toThrow(InvalidArgumentException::class); +}); + +test('an unknown concurrency mode is refused', function () { + expect(fn () => SystemTaskRegistry::register('pkg.temp', RegistryTestHandler::class, ['mode' => 'whenever'])) + ->toThrow(InvalidArgumentException::class); +}); + +test('a built-in type cannot be redefined or forgotten', function () { + expect(fn () => SystemTaskRegistry::register('site_update', RegistryTestHandler::class)) + ->toThrow(InvalidArgumentException::class); + + SystemTaskRegistry::forget('site_update'); + + expect(SystemTaskRegistry::has('site_update'))->toBeTrue(); +}); + +test('an exclusive registration cannot claim a parallelism', function () { + SystemTaskRegistry::register('pkg.exclusive', RegistryTestHandler::class, ['parallelism' => 9]); + + expect(SystemTaskRegistry::parallelism('pkg.exclusive'))->toBe(1) + ->and(SystemTaskRegistry::parallelism('pkg.batch'))->toBe(3); +}); + +test('a handler resolves from a class string, an instance or a factory', function () { + SystemTaskRegistry::register('pkg.instance', new RegistryTestHandler()); + SystemTaskRegistry::register('pkg.factory', fn () => new RegistryTestHandler()); + + expect(SystemTaskRegistry::handler('pkg.batch'))->toBeInstanceOf(RegistryTestHandler::class) + ->and(SystemTaskRegistry::handler('pkg.instance'))->toBeInstanceOf(RegistryTestHandler::class) + ->and(SystemTaskRegistry::handler('pkg.factory'))->toBeInstanceOf(RegistryTestHandler::class); +}); + +test('a registered class that is not a handler is refused when resolved', function () { + SystemTaskRegistry::register('pkg.bogus', stdClass::class); + + expect(fn () => SystemTaskRegistry::handler('pkg.bogus'))->toThrow(InvalidArgumentException::class); + expect(fn () => SystemTaskRegistry::handler('nope'))->toThrow(InvalidArgumentException::class); +}); + +test('exclusiveTypes omits concurrent registrations', function () { + SystemTaskRegistry::register('pkg.exclusive', RegistryTestHandler::class); + + $types = SystemTaskRegistry::exclusiveTypes(); + + expect($types)->toContain('site_update') + ->and($types)->toContain('pkg.exclusive') + ->and($types)->not->toContain('pkg.batch'); +}); + +// --------------------------------------------------------------------------- +// Queue concurrency +// --------------------------------------------------------------------------- + +test('a concurrent task is picked while an exclusive one is running', function () { + makeRegistryTask('site_update', 'running', Carbon::now()->addMinutes(10)); + $queued = makeRegistryTask('pkg.batch', 'queued'); + + $picked = (new SystemTaskService())->acquireNextQueuedTask('worker-1'); + + expect($picked)->not->toBeNull() + ->and((int) $picked->id)->toBe((int) $queued->id); +}); + +test('an exclusive task waits while another exclusive one is running', function () { + makeRegistryTask('site_update', 'running', Carbon::now()->addMinutes(10)); + makeRegistryTask('console_install', 'queued'); + + expect((new SystemTaskService())->acquireNextQueuedTask('worker-1'))->toBeNull(); +}); + +test('a blocked exclusive task does not stall a concurrent one behind it', function () { + makeRegistryTask('site_update', 'running', Carbon::now()->addMinutes(10)); + makeRegistryTask('console_install', 'queued'); + $batch = makeRegistryTask('pkg.batch', 'queued'); + + $picked = (new SystemTaskService())->acquireNextQueuedTask('worker-1'); + + expect($picked)->not->toBeNull() + ->and((int) $picked->id)->toBe((int) $batch->id); +}); + +test('an expired lease stops holding the queue', function () { + makeRegistryTask('site_update', 'picked', Carbon::now()->subMinutes(30)); + $queued = makeRegistryTask('console_install', 'queued'); + + $picked = (new SystemTaskService())->acquireNextQueuedTask('worker-1'); + + expect($picked)->not->toBeNull() + ->and((int) $picked->id)->toBe((int) $queued->id); +}); + +test('parallelism caps how many tasks of one concurrent type run at once', function () { + makeRegistryTask('pkg.batch', 'running', Carbon::now()->addMinutes(10)); + makeRegistryTask('pkg.batch', 'running', Carbon::now()->addMinutes(10)); + makeRegistryTask('pkg.batch', 'running', Carbon::now()->addMinutes(10)); + makeRegistryTask('pkg.batch', 'queued'); + + expect((new SystemTaskService())->acquireNextQueuedTask('worker-1'))->toBeNull(); +}); + +test('a free slot under the parallelism cap is used', function () { + makeRegistryTask('pkg.batch', 'running', Carbon::now()->addMinutes(10)); + makeRegistryTask('pkg.batch', 'running', Carbon::now()->addMinutes(10)); + $queued = makeRegistryTask('pkg.batch', 'queued'); + + $picked = (new SystemTaskService())->acquireNextQueuedTask('worker-1'); + + expect($picked)->not->toBeNull() + ->and((int) $picked->id)->toBe((int) $queued->id); +}); + +test('a task whose package is gone is still picked, so the worker can fail it', function () { + $queued = makeRegistryTask('gone.package', 'queued'); + + $picked = (new SystemTaskService())->acquireNextQueuedTask('worker-1'); + + expect($picked)->not->toBeNull() + ->and((int) $picked->id)->toBe((int) $queued->id); +}); + +test('only one worker can claim a task', function () { + makeRegistryTask('console_install', 'queued'); + + $service = new SystemTaskService(); + $first = $service->acquireNextQueuedTask('worker-1'); + $second = $service->acquireNextQueuedTask('worker-2'); + + expect($first)->not->toBeNull() + ->and($first->status)->toBe('picked') + ->and((int) $first->attempt_count)->toBe(1) + ->and($first->lease_expires_at)->not->toBeNull() + ->and($second)->toBeNull(); +}); + +// --------------------------------------------------------------------------- +// Preflight +// --------------------------------------------------------------------------- + +test('an unknown type is refused before any health verdict is disclosed', function () { + $result = runRegistryPreflight(new SystemTaskService(), 'gone.package', registryAdminSnapshot(), true); + + expect($result['ok'])->toBeFalse() + ->and($result['error_code'])->toBe('TASK_TYPE_NOT_ALLOWED'); +}); + +test('a package type is gated by the permission it declared', function () { + $snapshot = ['user_id' => 7, 'permissions' => ['exec_module' => true]]; + + $result = runRegistryPreflight(new SystemTaskService(), 'pkg.batch', $snapshot); + + expect($result['ok'])->toBeFalse() + ->and($result['error_code'])->toBe('ACL_DENIED') + ->and($result['message'])->toContain('pkg.run'); +}); + +test('a queued exclusive task still blocks another exclusive one', function () { + makeRegistryTask('site_update', 'queued'); + + $result = runRegistryPreflight(new SystemTaskService(), 'console_install', registryAdminSnapshot()); + + expect($result['ok'])->toBeFalse() + ->and($result['error_code'])->toBe('GLOBAL_LOCK_ACTIVE'); +}); + +test('an exclusive task no longer blocks a concurrent one', function () { + makeRegistryTask('site_update', 'queued'); + + $result = runRegistryPreflight(new SystemTaskService(), 'pkg.batch', registryAdminSnapshot()); + + // Scheduler health is unseeded here, so the call is expected to stop on a + // health verdict. What matters is that it got past the global lock. + expect($result['error_code'] ?? '')->not->toBe('GLOBAL_LOCK_ACTIVE'); +}); + +test('a running concurrent task no longer blocks an exclusive one', function () { + makeRegistryTask('pkg.batch', 'running', Carbon::now()->addMinutes(10)); + + $result = runRegistryPreflight(new SystemTaskService(), 'console_install', registryAdminSnapshot()); + + expect($result['error_code'] ?? '')->not->toBe('GLOBAL_LOCK_ACTIVE'); +}); diff --git a/core/tests/Unit/SystemTasks/SystemTaskServiceTest.php b/core/tests/Unit/SystemTasks/SystemTaskServiceTest.php index 3a83ce888b..b87848aa44 100644 --- a/core/tests/Unit/SystemTasks/SystemTaskServiceTest.php +++ b/core/tests/Unit/SystemTasks/SystemTaskServiceTest.php @@ -1,13 +1,26 @@ delete(); \EvolutionCMS\Models\SystemSchedulerHealth::query()->delete(); \EvolutionCMS\Models\SystemWorkerHealth::query()->delete(); + SystemTaskRegistry::forget('custom.worker_test'); +}); + +afterEach(function () { + SystemTaskRegistry::forget('custom.worker_test'); }); function invokeSystemTaskServiceMethod(SystemTaskService $service, string $method, array $args = []) @@ -747,3 +765,69 @@ public function getConsoleCatalog() expect($response['ok'])->toBeFalse() ->and($response['error_code'])->toBe('ACL_DENIED'); }); + +test('task worker executes registered extension task handlers', function () { + $handler = new class implements SystemTaskHandlerInterface { + public function execute(SystemCliTask $task, ?callable $report = null) + { + if ($report !== null) { + $report('extension_step', 40, 'Extension task is running.', 'info', [ + 'target' => (string) $task->target, + ]); + } + + return [ + 'message' => 'Extension task completed.', + 'result' => [ + 'handled_by' => 'extension', + 'target' => (string) $task->target, + ], + ]; + } + }; + + SystemTaskRegistry::register('custom.worker_test', $handler, [ + 'label' => 'Custom worker test', + ]); + + $task = SystemCliTask::query()->create([ + 'uuid' => 'custom-worker-task', + 'type' => 'custom.worker_test', + 'target' => 'custom-target', + 'requested_version' => '', + 'status' => 'queued', + 'step' => 'queued', + 'progress' => 0, + 'message' => 'Queued', + 'payload_json' => ['display_title' => 'Custom worker task'], + 'result_json' => [], + 'created_by' => 7, + 'locked_by' => '', + 'attempt_count' => 0, + 'worker_host' => '', + 'worker_pid' => null, + 'error_code' => '', + 'catalog_snapshot_hash' => '', + 'requested_by_snapshot' => ['user_id' => 7], + 'created_at' => Carbon::now(), + 'updated_at' => Carbon::now(), + ]); + + $command = new TaskWorkerCommand(); + $command->setLaravel(new SystemTaskWorkerTestContainer()); + + $tester = new CommandTester($command); + $exitCode = $tester->execute(['--once' => true]); + + $task->refresh(); + + expect($exitCode)->toBe(0) + ->and($task->status)->toBe('succeeded') + ->and($task->progress)->toBe(100) + ->and($task->message)->toBe('Extension task completed.') + ->and($task->result_json)->toBe([ + 'handled_by' => 'extension', + 'target' => 'custom-target', + ]) + ->and($tester->getDisplay())->toContain('[system:task-worker] custom.worker_test task completed'); +}); diff --git a/core/vendor/composer/autoload_classmap.php b/core/vendor/composer/autoload_classmap.php index fd81d68b36..91683a5dec 100644 --- a/core/vendor/composer/autoload_classmap.php +++ b/core/vendor/composer/autoload_classmap.php @@ -1254,6 +1254,7 @@ 'EvolutionCMS\\Interfaces\\PasswordHashInterface' => $baseDir . '/src/Interfaces/PasswordHashInterface.php', 'EvolutionCMS\\Interfaces\\PhpCompatInterface' => $baseDir . '/src/Interfaces/PhpCompatInterface.php', 'EvolutionCMS\\Interfaces\\ServiceProviderInterface' => $baseDir . '/src/Interfaces/ServiceProviderInterface.php', + 'EvolutionCMS\\Interfaces\\SystemTaskHandlerInterface' => $baseDir . '/src/Interfaces/SystemTaskHandlerInterface.php', 'EvolutionCMS\\Interfaces\\TracyPanel' => $baseDir . '/src/Interfaces/TracyPanel.php', 'EvolutionCMS\\Interfaces\\UserManagerInterface' => $baseDir . '/src/Interfaces/UserManagerInterface.php', 'EvolutionCMS\\Legacy\\Cache' => $baseDir . '/src/Legacy/Cache.php', @@ -1380,10 +1381,12 @@ 'EvolutionCMS\\Services\\SystemTasks\\ConsoleUninstall\\ConsoleUninstallResult' => $baseDir . '/src/Services/SystemTasks/ConsoleUninstall/ConsoleUninstallResult.php', 'EvolutionCMS\\Services\\SystemTasks\\SchedulerHealthService' => $baseDir . '/src/Services/SystemTasks/SchedulerHealthService.php', 'EvolutionCMS\\Services\\SystemTasks\\SiteUpdateFlowService' => $baseDir . '/src/Services/SystemTasks/SiteUpdateFlowService.php', + 'EvolutionCMS\\Services\\SystemTasks\\SystemTaskRegistry' => $baseDir . '/src/Services/SystemTasks/SystemTaskRegistry.php', 'EvolutionCMS\\Services\\SystemTasks\\SystemTaskService' => $baseDir . '/src/Services/SystemTasks/SystemTaskService.php', 'EvolutionCMS\\Services\\SystemTasks\\WorkerHealthService' => $baseDir . '/src/Services/SystemTasks/WorkerHealthService.php', 'EvolutionCMS\\Services\\TailwindService' => $baseDir . '/src/Services/TailwindService.php', 'EvolutionCMS\\Shit\\SoftDeletingScope' => $baseDir . '/src/Shit/SoftDeletingScope.php', + 'EvolutionCMS\\Support\\ArithmeticExpression' => $baseDir . '/src/Support/ArithmeticExpression.php', 'EvolutionCMS\\Support\\BladeDirective' => $baseDir . '/src/Support/BladeDirective.php', 'EvolutionCMS\\Support\\Captcha' => $baseDir . '/src/Support/Captcha.php', 'EvolutionCMS\\Support\\ContextMenu' => $baseDir . '/src/Support/ContextMenu.php', diff --git a/core/vendor/composer/autoload_static.php b/core/vendor/composer/autoload_static.php index 955f102107..16b376b8b1 100644 --- a/core/vendor/composer/autoload_static.php +++ b/core/vendor/composer/autoload_static.php @@ -1931,6 +1931,7 @@ class ComposerStaticInit925fea465a58fa69f06ccf2629003e87 'EvolutionCMS\\Interfaces\\PasswordHashInterface' => __DIR__ . '/../..' . '/src/Interfaces/PasswordHashInterface.php', 'EvolutionCMS\\Interfaces\\PhpCompatInterface' => __DIR__ . '/../..' . '/src/Interfaces/PhpCompatInterface.php', 'EvolutionCMS\\Interfaces\\ServiceProviderInterface' => __DIR__ . '/../..' . '/src/Interfaces/ServiceProviderInterface.php', + 'EvolutionCMS\\Interfaces\\SystemTaskHandlerInterface' => __DIR__ . '/../..' . '/src/Interfaces/SystemTaskHandlerInterface.php', 'EvolutionCMS\\Interfaces\\TracyPanel' => __DIR__ . '/../..' . '/src/Interfaces/TracyPanel.php', 'EvolutionCMS\\Interfaces\\UserManagerInterface' => __DIR__ . '/../..' . '/src/Interfaces/UserManagerInterface.php', 'EvolutionCMS\\Legacy\\Cache' => __DIR__ . '/../..' . '/src/Legacy/Cache.php', @@ -2057,10 +2058,12 @@ class ComposerStaticInit925fea465a58fa69f06ccf2629003e87 'EvolutionCMS\\Services\\SystemTasks\\ConsoleUninstall\\ConsoleUninstallResult' => __DIR__ . '/../..' . '/src/Services/SystemTasks/ConsoleUninstall/ConsoleUninstallResult.php', 'EvolutionCMS\\Services\\SystemTasks\\SchedulerHealthService' => __DIR__ . '/../..' . '/src/Services/SystemTasks/SchedulerHealthService.php', 'EvolutionCMS\\Services\\SystemTasks\\SiteUpdateFlowService' => __DIR__ . '/../..' . '/src/Services/SystemTasks/SiteUpdateFlowService.php', + 'EvolutionCMS\\Services\\SystemTasks\\SystemTaskRegistry' => __DIR__ . '/../..' . '/src/Services/SystemTasks/SystemTaskRegistry.php', 'EvolutionCMS\\Services\\SystemTasks\\SystemTaskService' => __DIR__ . '/../..' . '/src/Services/SystemTasks/SystemTaskService.php', 'EvolutionCMS\\Services\\SystemTasks\\WorkerHealthService' => __DIR__ . '/../..' . '/src/Services/SystemTasks/WorkerHealthService.php', 'EvolutionCMS\\Services\\TailwindService' => __DIR__ . '/../..' . '/src/Services/TailwindService.php', 'EvolutionCMS\\Shit\\SoftDeletingScope' => __DIR__ . '/../..' . '/src/Shit/SoftDeletingScope.php', + 'EvolutionCMS\\Support\\ArithmeticExpression' => __DIR__ . '/../..' . '/src/Support/ArithmeticExpression.php', 'EvolutionCMS\\Support\\BladeDirective' => __DIR__ . '/../..' . '/src/Support/BladeDirective.php', 'EvolutionCMS\\Support\\Captcha' => __DIR__ . '/../..' . '/src/Support/Captcha.php', 'EvolutionCMS\\Support\\ContextMenu' => __DIR__ . '/../..' . '/src/Support/ContextMenu.php',