Sort Rules: Added creation hints to sort rule selection

To help direct/indicate how rules can be created.
For #5967

.github/workflows/test-js.yml

@@ -17,7 +17,7 @@ jobs:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
     runs-on: ubuntu-24.04
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v6
 
     - name: Install NPM deps
       run: npm ci

.gitignore

@@ -2,6 +2,7 @@
 /node_modules
 /.vscode
 /composer
+/composer.phar
 /coverage
 Homestead.yaml
 .env

app/Access/EmailConfirmationService.php

@@ -5,6 +5,7 @@ namespace BookStack\Access;
 use BookStack\Access\Notifications\ConfirmEmailNotification;
 use BookStack\Exceptions\ConfirmationEmailException;
 use BookStack\Users\Models\User;
+use Exception;
 
 class EmailConfirmationService extends UserTokenService
 {
@@ -16,6 +17,7 @@ class EmailConfirmationService extends UserTokenService
      * Also removes any existing old ones.
      *
      * @throws ConfirmationEmailException
+     * @throws Exception
      */
     public function sendConfirmation(User $user): void
     {

app/Access/LoginService.php

@@ -71,7 +71,7 @@ class LoginService
         }
 
         $lastLoginDetails = $this->getLastLoginAttemptDetails();
-        $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember'] ?? false);
+        $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember']);
     }
 
     /**

app/Access/Mfa/MfaValue.php

@@ -48,17 +48,16 @@ class MfaValue extends Model
     }
 
     /**
-     * Easily get the decrypted MFA value for the given user and method.
+     * Get the decrypted MFA value for the given user and method.
      */
     public static function getValueForUser(User $user, string $method): ?string
     {
-        /** @var MfaValue $mfaVal */
         $mfaVal = static::query()
             ->where('user_id', '=', $user->id)
             ->where('method', '=', $method)
             ->first();
 
-        return $mfaVal ? $mfaVal->getValue() : null;
+        return $mfaVal?->getValue();
     }
 
     /**

app/Access/Oidc/OidcJwtSigningKey.php

@@ -9,10 +9,7 @@ use phpseclib3\Math\BigInteger;
 
 class OidcJwtSigningKey
 {
-    /**
-     * @var PublicKey
-     */
-    protected $key;
+    protected PublicKey $key;
 
     /**
      * Can be created either from a JWK parameter array or local file path to load a certificate from.
@@ -20,15 +17,13 @@ class OidcJwtSigningKey
      * 'file:///var/www/cert.pem'
      * ['kty' => 'RSA', 'alg' => 'RS256', 'n' => 'abc123...'].
      *
-     * @param array|string $jwkOrKeyPath
-     *
      * @throws OidcInvalidKeyException
      */
-    public function __construct($jwkOrKeyPath)
+    public function __construct(array|string $jwkOrKeyPath)
     {
         if (is_array($jwkOrKeyPath)) {
             $this->loadFromJwkArray($jwkOrKeyPath);
-        } elseif (is_string($jwkOrKeyPath) && strpos($jwkOrKeyPath, 'file://') === 0) {
+        } elseif (str_starts_with($jwkOrKeyPath, 'file://')) {
             $this->loadFromPath($jwkOrKeyPath);
         } else {
             throw new OidcInvalidKeyException('Unexpected type of key value provided');
@@ -38,7 +33,7 @@ class OidcJwtSigningKey
     /**
      * @throws OidcInvalidKeyException
      */
-    protected function loadFromPath(string $path)
+    protected function loadFromPath(string $path): void
     {
         try {
             $key = PublicKeyLoader::load(
@@ -58,7 +53,7 @@ class OidcJwtSigningKey
     /**
      * @throws OidcInvalidKeyException
      */
-    protected function loadFromJwkArray(array $jwk)
+    protected function loadFromJwkArray(array $jwk): void
     {
         // 'alg' is optional for a JWK, but we will still attempt to validate if
         // it exists otherwise presume it will be compatible.
@@ -82,7 +77,7 @@ class OidcJwtSigningKey
             throw new OidcInvalidKeyException('A "n" parameter on the provided key is expected');
         }
 
-        $n = strtr($jwk['n'] ?? '', '-_', '+/');
+        $n = strtr($jwk['n'], '-_', '+/');
 
         try {
             $key = PublicKeyLoader::load([

app/Access/Oidc/OidcJwtWithClaims.php

@@ -102,12 +102,12 @@ class OidcJwtWithClaims implements ProvidesClaims
     protected function validateTokenStructure(): void
     {
         foreach (['header', 'payload'] as $prop) {
-            if (empty($this->$prop) || !is_array($this->$prop)) {
+            if (empty($this->$prop)) {
                 throw new OidcInvalidTokenException("Could not parse out a valid {$prop} within the provided token");
             }
         }
 
-        if (empty($this->signature) || !is_string($this->signature)) {
+        if (empty($this->signature)) {
             throw new OidcInvalidTokenException('Could not parse out a valid signature within the provided token');
         }
     }

app/Access/Oidc/OidcUserDetails.php

@@ -39,7 +39,7 @@ class OidcUserDetails
     ): void {
         $this->externalId = $claims->getClaim($idClaim) ?? $this->externalId;
         $this->email = $claims->getClaim('email') ?? $this->email;
-        $this->name = static::getUserDisplayName($displayNameClaims, $claims) ?? $this->name;
+        $this->name = static::getUserDisplayName($displayNameClaims, $claims) ?: $this->name;
         $this->groups = static::getUserGroups($groupsClaim, $claims) ?? $this->groups;
         $this->picture = static::getPicture($claims) ?: $this->picture;
     }

app/Access/Saml2Service.php

@@ -266,7 +266,7 @@ class Saml2Service
     /**
      * Extract the details of a user from a SAML response.
      *
-     * @return array{external_id: string, name: string, email: string, saml_id: string}
+     * @return array{external_id: string, name: string, email: string|null, saml_id: string}
      */
     protected function getUserDetails(string $samlID, $samlAttributes): array
     {
@@ -357,7 +357,7 @@ class Saml2Service
             ]);
         }
 
-        if ($userDetails['email'] === null) {
+        if (empty($userDetails['email'])) {
             throw new SamlException(trans('errors.saml_no_email_address'));
         }
 

app/Access/SocialAuthService.php

@@ -117,14 +117,14 @@ class SocialAuthService
         }
 
         // When a user is logged in and the social account exists and is already linked to the current user.
-        if ($isLoggedIn && $socialAccount !== null && $socialAccount->user->id === $currentUser->id) {
+        if ($isLoggedIn && $socialAccount->user->id === $currentUser->id) {
             session()->flash('error', trans('errors.social_account_existing', ['socialAccount' => $titleCaseDriver]));
 
             return redirect('/my-account/auth#social_accounts');
         }
 
         // When a user is logged in, A social account exists but the users do not match.
-        if ($isLoggedIn && $socialAccount !== null && $socialAccount->user->id != $currentUser->id) {
+        if ($isLoggedIn && $socialAccount->user->id != $currentUser->id) {
             session()->flash('error', trans('errors.social_account_already_used_existing', ['socialAccount' => $titleCaseDriver]));
 
             return redirect('/my-account/auth#social_accounts');

app/Activity/Notifications/NotificationManager.php

@@ -15,14 +15,14 @@ use BookStack\Users\Models\User;
 class NotificationManager
 {
     /**
-     * @var class-string<NotificationHandler>[]
+     * @var array<string, class-string<NotificationHandler>[]>
      */
-    protected array $handlers = [];
+    protected array $handlersByActivity = [];
 
     public function handle(Activity $activity, string|Loggable $detail, User $user): void
     {
         $activityType = $activity->type;
-        $handlersToRun = $this->handlers[$activityType] ?? [];
+        $handlersToRun = $this->handlersByActivity[$activityType] ?? [];
         foreach ($handlersToRun as $handlerClass) {
             /** @var NotificationHandler $handler */
             $handler = new $handlerClass();
@@ -35,12 +35,12 @@ class NotificationManager
      */
     public function registerHandler(string $activityType, string $handlerClass): void
     {
-        if (!isset($this->handlers[$activityType])) {
-            $this->handlers[$activityType] = [];
+        if (!isset($this->handlersByActivity[$activityType])) {
+            $this->handlersByActivity[$activityType] = [];
         }
 
-        if (!in_array($handlerClass, $this->handlers[$activityType])) {
-            $this->handlers[$activityType][] = $handlerClass;
+        if (!in_array($handlerClass, $this->handlersByActivity[$activityType])) {
+            $this->handlersByActivity[$activityType][] = $handlerClass;
         }
     }
 

app/Api/ApiDocsGenerator.php

@@ -17,7 +17,14 @@ use ReflectionMethod;
 
 class ApiDocsGenerator
 {
+    /**
+     * @var array<string, ReflectionClass>
+     */
     protected array $reflectionClasses = [];
+
+    /**
+     * @var array<string, ApiController>
+     */
     protected array $controllerClasses = [];
 
     /**
@@ -107,7 +114,6 @@ class ApiDocsGenerator
      */
     protected function getBodyParamsFromClass(string $className, string $methodName): ?array
     {
-        /** @var ApiController $class */
         $class = $this->controllerClasses[$className] ?? null;
         if ($class === null) {
             $class = app()->make($className);
@@ -153,7 +159,7 @@ class ApiDocsGenerator
         $matches = [];
         preg_match_all('/^\s*?\*\s?($|((?![\/@\s]).*?))$/m', $comment, $matches);
 
-        $text = implode(' ', $matches[1] ?? []);
+        $text = implode(' ', $matches[1]);
         return str_replace('  ', "\n", $text);
     }
 

app/Api/ApiEntityListFormatter.php

@@ -74,18 +74,21 @@ class ApiEntityListFormatter
 
     /**
      * Include parent book/chapter info in the formatted data.
+     * These functions are careful to not load the relation themselves, since they should
+     * have already been loaded in a more efficient manner, with permissions applied, by the time
+     * the parent fields are handled here.
      */
     public function withParents(): self
     {
         $this->withField('book', function (Entity $entity) {
-            if ($entity instanceof BookChild && $entity->book) {
+            if ($entity instanceof BookChild && $entity->relationLoaded('book') && $entity->getRelationValue('book')) {
                 return $entity->book->only(['id', 'name', 'slug']);
             }
             return null;
         });
 
         $this->withField('chapter', function (Entity $entity) {
-            if ($entity instanceof Page && $entity->chapter) {
+            if ($entity instanceof Page && $entity->relationLoaded('chapter') && $entity->getRelationValue('chapter')) {
                 return $entity->chapter->only(['id', 'name', 'slug']);
             }
             return null;

app/Api/ApiTokenGuard.php

@@ -16,30 +16,15 @@ class ApiTokenGuard implements Guard
 {
     use GuardHelpers;
 
-    /**
-     * The request instance.
-     */
-    protected $request;
-
-    /**
-     * @var LoginService
-     */
-    protected $loginService;
-
     /**
      * The last auth exception thrown in this request.
-     *
-     * @var ApiAuthException
      */
-    protected $lastAuthException;
+    protected ApiAuthException|null $lastAuthException = null;
 
-    /**
-     * ApiTokenGuard constructor.
-     */
-    public function __construct(Request $request, LoginService $loginService)
-    {
-        $this->request = $request;
-        $this->loginService = $loginService;
+    public function __construct(
+        protected Request $request,
+        protected LoginService $loginService
+    ) {
     }
 
     /**
@@ -67,7 +52,7 @@ class ApiTokenGuard implements Guard
     }
 
     /**
-     * Determine if current user is authenticated. If not, throw an exception.
+     * Determine if the current user is authenticated. If not, throw an exception.
      *
      * @throws ApiAuthException
      *
@@ -121,7 +106,7 @@ class ApiTokenGuard implements Guard
             throw new ApiAuthException(trans('errors.api_no_authorization_found'));
         }
 
-        if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) {
+        if (!str_contains($authToken, ':') || !str_starts_with($authToken, 'Token ')) {
             throw new ApiAuthException(trans('errors.api_bad_authorization_format'));
         }
     }
@@ -155,7 +140,7 @@ class ApiTokenGuard implements Guard
     /**
      * {@inheritdoc}
      */
-    public function validate(array $credentials = [])
+    public function validate(array $credentials = []): bool
     {
         if (empty($credentials['id']) || empty($credentials['secret'])) {
             return false;
@@ -175,7 +160,7 @@ class ApiTokenGuard implements Guard
     /**
      * "Log out" the currently authenticated user.
      */
-    public function logout()
+    public function logout(): void
     {
         $this->user = null;
     }

app/Console/Commands/AssignSortRuleCommand.php

@@ -32,7 +32,7 @@ class AssignSortRuleCommand extends Command
      */
     public function handle(BookSorter $sorter): int
     {
-        $sortRuleId = intval($this->argument('sort-rule')) ?? 0;
+        $sortRuleId = intval($this->argument('sort-rule'));
         if ($sortRuleId === 0) {
             return $this->listSortRules();
         }

app/Console/Commands/CopyShelfPermissionsCommand.php

@@ -32,6 +32,7 @@ class CopyShelfPermissionsCommand extends Command
     {
         $shelfSlug = $this->option('slug');
         $cascadeAll = $this->option('all');
+        $noInteraction = boolval($this->option('no-interaction'));
         $shelves = null;
 
         if (!$cascadeAll && !$shelfSlug) {
@@ -41,14 +42,16 @@ class CopyShelfPermissionsCommand extends Command
         }
 
         if ($cascadeAll) {
-            $continue = $this->confirm(
-                'Permission settings for all shelves will be cascaded. ' .
-                        'Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. ' .
-                        'Are you sure you want to proceed?'
-            );
+            if (!$noInteraction) {
+                $continue = $this->confirm(
+                    'Permission settings for all shelves will be cascaded. ' .
+                    'Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. ' .
+                    'Are you sure you want to proceed?',
+                );
 
-            if (!$continue && !$this->hasOption('no-interaction')) {
-                return 0;
+                if (!$continue) {
+                    return 0;
+                }
             }
 
             $shelves = $queries->start()->get(['id']);

app/Console/Commands/InstallModuleCommand.php

@@ -213,15 +213,23 @@ class InstallModuleCommand extends Command
                 $redirectLocation = $resp->getHeaderLine('Location');
                 if ($redirectLocation) {
                     $redirectUrl = parse_url($redirectLocation);
-                    if (
-                        ($originalUrl['host'] ?? '') === ($redirectUrl['host'] ?? '')
+                    $redirectOriginMatches = ($originalUrl['host'] ?? '') === ($redirectUrl['host'] ?? '')
                         && ($originalUrl['scheme'] ?? '') === ($redirectUrl['scheme'] ?? '')
-                        && ($originalUrl['port'] ?? '') === ($redirectUrl['port'] ?? '')
-                    ) {
-                        $currentLocation = $redirectLocation;
-                        $redirectCount++;
-                        continue;
+                        && ($originalUrl['port'] ?? '') === ($redirectUrl['port'] ?? '');
+
+                    if (!$redirectOriginMatches) {
+                        $redirectOrigin = ($redirectUrl['scheme'] ?? '') . '://' . ($redirectUrl['host'] ?? '') . (isset($redirectUrl['port']) ? ':' . $redirectUrl['port'] : '');
+                        $this->info("The download URL is redirecting to a different site: {$redirectOrigin}");
+                        $shouldContinue = $this->confirm("Do you trust downloading the module from this site?");
+                        if (!$shouldContinue) {
+                            $this->error("Stopping module installation");
+                            return null;
+                        }
                     }
+
+                    $currentLocation = $redirectLocation;
+                    $redirectCount++;
+                    continue;
                 }
             }
 

app/Entities/Models/Book.php

@@ -17,7 +17,7 @@ use Illuminate\Support\Collection;
  *
  * @property string                                   $description
  * @property string                                   $description_html
- * @property int                                      $image_id
+ * @property ?int                                     $image_id
  * @property ?int                                     $default_template_id
  * @property ?int                                     $sort_rule_id
  * @property \Illuminate\Database\Eloquent\Collection $chapters

app/Entities/Models/Entity.php

@@ -479,6 +479,7 @@ abstract class Entity extends Model implements
             'chapter' => new Chapter(),
             'book' => new Book(),
             'bookshelf' => new Bookshelf(),
+            default => throw new \InvalidArgumentException("Invalid entity type: {$type}"),
         };
     }
 }

app/Entities/Models/Page.php

@@ -23,7 +23,7 @@ use Illuminate\Database\Eloquent\Relations\HasOne;
  * @property bool         $draft
  * @property int          $revision_count
  * @property string       $editor
- * @property Chapter      $chapter
+ * @property Chapter|null $chapter
  * @property Collection   $attachments
  * @property Collection   $revisions
  * @property PageRevision $currentRevision

app/Entities/Repos/PageRepo.php

@@ -60,7 +60,7 @@ class PageRepo
             $page->book_id = $parent->id;
         }
 
-        $defaultTemplate = $page->chapter?->defaultTemplate()->get() ?? $page->book?->defaultTemplate()->get();
+        $defaultTemplate = $page->chapter?->defaultTemplate()->get() ?? $page->book->defaultTemplate()->get();
         if ($defaultTemplate) {
             $page->forceFill([
                 'html'  => $defaultTemplate->html,

app/Entities/Tools/PageContent.php

@@ -359,7 +359,7 @@ class PageContent
     {
         $contentHash = md5($html);
         $contentId = $this->page->id;
-        $contentTime = $this->page->updated_at?->timestamp ?? time();
+        $contentTime = $this->page->updated_at->timestamp ?? time();
         $appVersion = AppVersion::get();
         $filterConfig = config('app.content_filtering') ?? '';
         return "page-content-cache::{$filterConfig}::{$appVersion}::{$contentId}::{$contentTime}::{$contentHash}";

app/Entities/Tools/PermissionsUpdater.php

@@ -47,7 +47,7 @@ class PermissionsUpdater
     {
         if (isset($data['role_permissions'])) {
             $entity->permissions()->where('role_id', '!=', 0)->delete();
-            $rolePermissionData = $this->formatPermissionsFromApiRequestToEntityPermissions($data['role_permissions'] ?? [], false);
+            $rolePermissionData = $this->formatPermissionsFromApiRequestToEntityPermissions($data['role_permissions'], false);
             $entity->permissions()->createMany($rolePermissionData);
         }
 

app/Exports/ExportFormatter.php

@@ -208,7 +208,7 @@ class ExportFormatter
         preg_match_all("/\<img.*?src\=(\'|\")(.*?)(\'|\").*?\>/i", $htmlContent, $imageTagsOutput);
 
         // Replace image src with base64 encoded image strings
-        if (isset($imageTagsOutput[0]) && count($imageTagsOutput[0]) > 0) {
+        if (count($imageTagsOutput[0]) > 0) {
             foreach ($imageTagsOutput[0] as $index => $imgMatch) {
                 $oldImgTagString = $imgMatch;
                 $srcString = $imageTagsOutput[2][$index];
@@ -225,7 +225,7 @@ class ExportFormatter
         preg_match_all("/\<a.*href\=(\'|\")(.*?)(\'|\").*?\>/i", $htmlContent, $linksOutput);
 
         // Update relative links to be absolute, with instance url
-        if (isset($linksOutput[0]) && count($linksOutput[0]) > 0) {
+        if (count($linksOutput[0]) > 0) {
             foreach ($linksOutput[0] as $index => $linkMatch) {
                 $oldLinkString = $linkMatch;
                 $srcString = $linksOutput[2][$index];

app/Exports/ZipExports/ZipImportRunner.php

@@ -82,10 +82,8 @@ class ZipImportRunner
             $entity = $this->importBook($exportModel, $reader);
         } else if ($exportModel instanceof ZipExportChapter) {
             $entity = $this->importChapter($exportModel, $parent, $reader);
-        } else if ($exportModel instanceof ZipExportPage) {
-            $entity = $this->importPage($exportModel, $parent, $reader);
         } else {
-            throw new ZipImportException(['No importable data found in import data.']);
+            $entity = $this->importPage($exportModel, $parent, $reader);
         }
 
         $this->references->replaceReferences();
@@ -132,7 +130,7 @@ class ZipImportRunner
             'name' => $exportBook->name,
             'description_html' => $exportBook->description_html ?? '',
             'image' => $exportBook->cover ? $this->zipFileToUploadedFile($exportBook->cover, $reader) : null,
-            'tags' => $this->exportTagsToInputArray($exportBook->tags ?? []),
+            'tags' => $this->exportTagsToInputArray($exportBook->tags),
         ]);
 
         if ($book->coverInfo()->getImage()) {
@@ -151,7 +149,7 @@ class ZipImportRunner
         foreach ($children as $child) {
             if ($child instanceof ZipExportChapter) {
                 $this->importChapter($child, $book, $reader);
-            } else if ($child instanceof ZipExportPage) {
+            } else {
                 $this->importPage($child, $book, $reader);
             }
         }
@@ -166,7 +164,7 @@ class ZipImportRunner
         $chapter = $this->chapterRepo->create([
             'name' => $exportChapter->name,
             'description_html' => $exportChapter->description_html ?? '',
-            'tags' => $this->exportTagsToInputArray($exportChapter->tags ?? []),
+            'tags' => $this->exportTagsToInputArray($exportChapter->tags),
         ], $parent);
 
         $exportPages = $exportChapter->pages;
@@ -199,7 +197,7 @@ class ZipImportRunner
             'name' => $exportPage->name,
             'markdown' => $exportPage->markdown ?? '',
             'html' => $exportPage->html ?? '',
-            'tags' => $this->exportTagsToInputArray($exportPage->tags ?? []),
+            'tags' => $this->exportTagsToInputArray($exportPage->tags),
         ]);
 
         $this->references->addPage($page, $exportPage);
@@ -302,7 +300,7 @@ class ZipImportRunner
             array_push($chapters, ...$exportModel->chapters);
         } else if ($exportModel instanceof ZipExportChapter) {
             $chapters[] = $exportModel;
-        } else if ($exportModel instanceof ZipExportPage) {
+        } else {
             $pages[] = $exportModel;
         }
 

app/Exports/ZipExports/ZipReferenceParser.php

@@ -68,10 +68,6 @@ class ZipReferenceParser
         $matches = [];
         preg_match_all($referenceRegex, $content, $matches);
 
-        if (count($matches) < 3) {
-            return $content;
-        }
-
         for ($i = 0; $i < count($matches[0]); $i++) {
             $referenceText = $matches[0][$i];
             $type = strtolower($matches[1][$i]);

app/Http/Controller.php

@@ -62,7 +62,7 @@ abstract class Controller extends BaseController
      */
     protected function checkPermission(string|Permission $permission): void
     {
-        if (!user() || !user()->can($permission)) {
+        if (!user()->can($permission)) {
             $this->showPermissionError();
         }
     }

app/Permissions/JointPermissionBuilder.php

@@ -61,8 +61,7 @@ class JointPermissionBuilder
             return;
         }
 
-        /** @var BookChild $entity */
-        if ($entity->book) {
+        if ($entity instanceof BookChild) {
             $entities[] = $entity->book;
         }
 

app/Search/SearchOptions.php

@@ -121,13 +121,11 @@ class SearchOptions
         foreach ($patterns as $termType => $pattern) {
             $matches = [];
             preg_match_all($pattern, $searchString, $matches);
-            if (count($matches) > 0) {
-                foreach ($matches[1] as $index => $value) {
-                    $negated = str_starts_with($matches[0][$index], '-');
-                    $terms[$termType][] = $constructors[$termType]($value, $negated);
-                }
-                $searchString = preg_replace($pattern, '', $searchString);
+            foreach ($matches[1] as $index => $value) {
+                $negated = str_starts_with($matches[0][$index], '-');
+                $terms[$termType][] = $constructors[$termType]($value, $negated);
             }
+            $searchString = preg_replace($pattern, '', $searchString);
         }
 
         // Unescape exacts and backslash escapes
@@ -261,7 +259,7 @@ class SearchOptions
         $userFilters = ['updated_by', 'created_by', 'owned_by'];
         $unsupportedFilters = ['is_template', 'sort_by'];
         foreach ($this->filters->all() as $filter) {
-            if (in_array($filter->getKey(), $userFilters, true) && $filter->value !== null && $filter->value !== 'me') {
+            if (in_array($filter->getKey(), $userFilters, true) && $filter->value && $filter->value !== 'me') {
                 $options[] = $filter;
             } else if (in_array($filter->getKey(), $unsupportedFilters, true)) {
                 $options[] = $filter;

app/Sorting/BookSorter.php

@@ -125,9 +125,8 @@ class BookSorter
      */
     protected function applySortUpdates(BookSortMapItem $sortMapItem, array $modelMap): void
     {
-        /** @var BookChild $model */
         $model = $modelMap[$sortMapItem->type . ':' . $sortMapItem->id] ?? null;
-        if (!$model) {
+        if (!($model instanceof BookChild)) {
             return;
         }
 

app/Theming/ThemeModuleManager.php

@@ -51,7 +51,14 @@ class ThemeModuleManager
         }
 
         $folderPath = $this->modulesFolderPath . DIRECTORY_SEPARATOR . $folderName;
-        $zip->extractTo($folderPath);
+        try {
+            $zip->extractTo($folderPath);
+        } catch (ThemeModuleException $exception) {
+            if (is_dir($folderPath)) {
+                $this->deleteDirectoryRecursively($folderPath);
+            }
+            throw new ThemeModuleException("Failed to load extract files from module ZIP with error: {$exception->getMessage()}");
+        }
 
         $module = $this->loadFromFolder($folderName);
         if (!$module) {

app/Theming/ThemeModuleZip.php

@@ -2,6 +2,7 @@
 
 namespace BookStack\Theming;
 
+use BookStack\Util\FilePathNormalizer;
 use ZipArchive;
 
 readonly class ThemeModuleZip
@@ -15,7 +16,46 @@ readonly class ThemeModuleZip
     {
         $zip = new ZipArchive();
         $zip->open($this->path);
-        $zip->extractTo($destinationPath);
+        $prefix = $this->getZipContentPrefix($zip);
+
+        for ($i = 0; $i < $zip->numFiles; $i++) {
+            $name = $zip->getNameIndex($i);
+            $entryIsDir = str_ends_with($name, "/");
+            if ($entryIsDir) {
+                continue;
+            }
+
+            $stream = $zip->getStreamIndex($i);
+
+            if ($prefix) {
+                if (!str_starts_with($name, $prefix) || $name === $prefix) {
+                    continue;
+                }
+                $name = str_replace($prefix, '', $name);
+            }
+
+            try {
+                $targetPath = $destinationPath . DIRECTORY_SEPARATOR . FilePathNormalizer::normalize($name);
+            } catch (\Exception $exception) {
+                throw new ThemeModuleException("Bad file path found in module ZIP file: {$name}");
+            }
+
+            $targetPathDir = dirname($targetPath);
+            if (!is_dir($targetPathDir)) {
+                $dirCreated = mkdir($targetPathDir, 0777, true);
+                if (!$dirCreated) {
+                    throw new ThemeModuleException("Failed to create directory {$targetPathDir} when extracting module files");
+                }
+            }
+
+            $targetFile = fopen($targetPath, 'w');
+            $written = stream_copy_to_stream($stream, $targetFile);
+            if (!$written) {
+                throw new ThemeModuleException("Failed to write to {$targetPath} when extracting module files");
+            }
+            fclose($targetFile);
+        }
+
         $zip->close();
     }
 
@@ -31,7 +71,8 @@ readonly class ThemeModuleZip
             throw new ThemeModuleException("Unable to open zip file at {$this->path}");
         }
 
-        $moduleJsonText = $zip->getFromName('bookstack-module.json');
+        $prefix = $this->getZipContentPrefix($zip);
+        $moduleJsonText = $zip->getFromName("{$prefix}bookstack-module.json");
         $zip->close();
 
         if ($moduleJsonText === false) {
@@ -95,4 +136,20 @@ readonly class ThemeModuleZip
 
         return $totalSize;
     }
+
+    protected function getZipContentPrefix(ZipArchive $zip): string
+    {
+        $index = $zip->locateName('bookstack-module.json', ZipArchive::FL_NODIR);
+        if ($index === false) {
+            return '';
+        }
+
+        $location = $zip->getNameIndex($index);
+        $pathParts = explode('/', $location);
+        if (count($pathParts) !== 2) {
+            return '';
+        }
+
+        return $pathParts[0] . '/';
+    }
 }

app/Uploads/ImageRepo.php

@@ -91,7 +91,7 @@ class ImageRepo
             $parentFilter = function (Builder $query) use ($filterType, $contextPage) {
                 if ($filterType === 'page') {
                     $query->where('uploaded_to', '=', $contextPage->id);
-                } else if ($filterType === 'book') {
+                } else {
                     $validPageIds = $contextPage->book->pages()
                         ->scopes('visible')
                         ->pluck('id')

app/Uploads/UserAvatars.php

@@ -148,7 +148,7 @@ class UserAvatars
                 $responseCount++;
                 $isRedirect = ($response->getStatusCode() === 301 || $response->getStatusCode() === 302);
                 $url = $response->getHeader('Location')[0] ?? '';
-            } while ($responseCount < 3 && $isRedirect && is_string($url) && str_starts_with($url, 'http'));
+            } while ($responseCount < 3 && $isRedirect && str_starts_with($url, 'http'));
 
             if ($responseCount === 3) {
                 throw new HttpFetchException("Failed to fetch image, max redirect limit of 3 tries reached. Last fetched URL: {$url}");

app/Users/Models/User.php

@@ -222,8 +222,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
     public function getAvatar(int $size = 50): string
     {
         $default = url('/user_avatar.png');
-        $imageId = $this->image_id;
-        if ($imageId === 0 || $imageId === '0' || $imageId === null) {
+        if ($this->image_id === 0) {
             return $default;
         }
 

dev/docs/theme-system-modules.md

@@ -66,6 +66,7 @@ Here are some general best practices when it comes to creating modules:
 ### Distribution Format
 
 Modules are expected to be distributed as a compressed ZIP file, where the ZIP contents follow that of a module folder.
+Contents may optionally be placed within a nested folder inside the ZIP.
 BookStack provides a `php artisan bookstack:install-module` command which allows modules to be installed from these ZIP files, either from a local path or from a web URL.
 Currently, there's a hardcoded total filesize limit of 50MB for module contents installed via this method.
 

jest.config.ts

@@ -5,7 +5,15 @@
 
 import type {Config} from 'jest';
 import {pathsToModuleNameMapper} from "ts-jest";
-import { compilerOptions }  from './tsconfig.json';
+import fs from "node:fs";
+
+const { compilerOptions } = JSON.parse(fs.readFileSync('./tsconfig.json', 'utf8'));
+const compilerPaths = compilerOptions.paths as Record<string, string[]>;
+const cleanedPaths: Record<string, string[]> = {};
+Object.keys(compilerPaths).forEach((key) => {
+  const paths = compilerPaths[key];
+    cleanedPaths[key] = paths.map(p => p.replace('./', ''));
+});
 
 const config: Config = {
   // All imported modules in your tests should be mocked automatically
@@ -98,7 +106,7 @@ const config: Config = {
   // A map from regular expressions to module names or to arrays of module names that allow to stub out resources with a single module
   moduleNameMapper: {
     'lexical/shared/invariant': 'resources/js/wysiwyg/lexical/core/shared/__mocks__/invariant',
-    ...pathsToModuleNameMapper(compilerOptions.paths),
+    ...pathsToModuleNameMapper(cleanedPaths),
   },
 
   // An array of regexp pattern strings, matched against all module paths before considered 'visible' to the module loader
@@ -111,7 +119,7 @@ const config: Config = {
   // notifyMode: "failure-change",
 
   // A preset that is used as a base for Jest's configuration
-  // preset: undefined,
+  preset: 'ts-jest',
 
   // Run tests from one or more projects
   // projects: undefined,

lang/en/entities.php

@@ -173,6 +173,7 @@ return [
     'books_sort_desc' => 'Move chapters and pages within a book to reorganise its contents. Other books can be added which allows easy moving of chapters and pages between books. Optionally an auto sort rule can be set to automatically sort this book\'s contents upon changes.',
     'books_sort_auto_sort' => 'Auto Sort Option',
     'books_sort_auto_sort_active' => 'Auto Sort Active: :sortName',
+    'books_sort_auto_sort_creation_hint' => 'Auto sort option rules can be created in the "Lists & Sorting" settings area by a user with the relevant permissions.',
     'books_sort_named' => 'Sort Book :bookName',
     'books_sort_name' => 'Sort by Name',
     'books_sort_created' => 'Sort by Created Date',

package.json

@@ -20,22 +20,23 @@
   "devDependencies": {
     "@eslint/js": "^10.0.1",
     "@lezer/generator": "^1.8.0",
+    "@types/jest": "^30.0.0",
     "@types/markdown-it": "^14.1.2",
     "@types/sortablejs": "^1.15.9",
     "chokidar-cli": "^3.0",
-    "esbuild": "^0.27.3",
-    "eslint": "^10.0.2",
+    "esbuild": "^0.28.0",
+    "eslint": "^10.2.0",
     "globals": "^17.4.0",
-    "jest": "^30.2.0",
-    "jest-environment-jsdom": "^30.2.0",
+    "jest": "^30.3.0",
+    "jest-environment-jsdom": "^30.3.0",
     "npm-run-all": "^4.1.5",
-    "sass": "^1.97.3",
-    "ts-jest": "^29.4.6",
+    "sass": "^1.99.0",
+    "ts-jest": "^29.4.9",
     "ts-node": "^10.9.2",
-    "typescript": "5.9.*"
+    "typescript": "6.0.*"
   },
   "dependencies": {
-    "@codemirror/commands": "^6.10.2",
+    "@codemirror/commands": "^6.10.3",
     "@codemirror/lang-css": "^6.3.1",
     "@codemirror/lang-html": "^6.4.11",
     "@codemirror/lang-javascript": "^6.2.5",
@@ -43,15 +44,14 @@
     "@codemirror/lang-markdown": "^6.5.0",
     "@codemirror/lang-php": "^6.0.2",
     "@codemirror/lang-xml": "^6.1.0",
-    "@codemirror/language": "^6.12.2",
+    "@codemirror/language": "^6.12.3",
     "@codemirror/legacy-modes": "^6.5.2",
-    "@codemirror/state": "^6.5.4",
+    "@codemirror/state": "^6.6.0",
     "@codemirror/theme-one-dark": "^6.1.3",
-    "@codemirror/view": "^6.39.16",
+    "@codemirror/view": "^6.41.0",
     "@lezer/highlight": "^1.2.3",
     "@ssddanbrown/codemirror-lang-smarty": "^1.0.0",
     "@ssddanbrown/codemirror-lang-twig": "^1.0.0",
-    "@types/jest": "^30.0.0",
     "codemirror": "^6.0.2",
     "idb-keyval": "^6.2.2",
     "markdown-it": "^14.1.1",

phpstan.neon.dist

@@ -7,11 +7,11 @@ parameters:
         - app
 
     # The level 8 is the highest level
-    level: 3
+    level: 4
 
     phpVersion:
         min: 80200
-        max: 80400
+        max: 80500
 
     bootstrapFiles:
       - bootstrap/phpstan.php

resources/views/books/sort.blade.php

@@ -20,7 +20,12 @@
                     <h1 class="list-heading">{{ trans('entities.books_sort') }}</h1>
 
                     <div class="flex-container-row gap-m wrap mb-m">
-                        <p class="text-muted flex min-width-s mb-none">{{ trans('entities.books_sort_desc') }}</p>
+                        <div class="flex min-width-s">
+                            <p class="text-muted  mb-none">{{ trans('entities.books_sort_desc') }}</p>
+                            @if(!userCan(\BookStack\Permissions\Permission::SettingsManage))
+                                <p class="text-muted mb-none mt-s small">{{ trans('entities.books_sort_auto_sort_creation_hint') }}</p>
+                            @endif
+                        </div>
                         <div class="min-width-s">
                             @php
                                 $autoSortVal = intval(old('auto-sort') ?? $book->sort_rule_id ?? 0);
@@ -41,6 +46,11 @@
                                     </option>
                                 @endforeach
                             </select>
+                            @if(userCan(\BookStack\Permissions\Permission::SettingsManage))
+                                <p class="small">
+                                    <a href="{{ url('/settings/sorting/rules/new') }}" class="text-button" target="_blank">{{ trans('settings.sort_rule_create') }}</a>
+                                </p>
+                            @endif
                         </div>
                     </div>
 

resources/views/settings/categories/sorting.blade.php

@@ -38,9 +38,9 @@
                 <div>
                     <label for="setting-sorting-book-default"
                            class="setting-list-label">{{ trans('settings.sorting_book_default') }}</label>
-                    <p class="small">{{ trans('settings.sorting_book_default_desc') }}</p>
+                    <p class="small mb-s">{{ trans('settings.sorting_book_default_desc') }}</p>
                 </div>
-                <div>
+                <div class="pt-s">
                     <select id="setting-sorting-book-default" name="setting-sorting-book-default"
                             @if($errors->has('setting-sorting-book-default')) class="neg" @endif>
                         <option value="0" @if(intval(setting('sorting-book-default', '0')) === 0) selected @endif>
@@ -54,6 +54,9 @@
                             </option>
                         @endforeach
                     </select>
+                    <p class="small">
+                        <a href="{{ url('/settings/sorting/rules/new') }}" class="text-button" target="_blank">{{ trans('settings.sort_rule_create') }}</a>
+                    </p>
                 </div>
             </div>
 

tests/Api/SearchApiTest.php

@@ -106,6 +106,7 @@ class SearchApiTest extends TestCase
         $this->permissions->setEntityPermissions($page, ['view'], [$editor->roles()->first()]);
 
         $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertOk();
         $resp->assertJsonPath('data.0.id', $page->id);
         $resp->assertJsonPath('data.0.book.name', $book->name);
         $resp->assertJsonMissingPath('data.0.chapter');

tests/Commands/CopyShelfPermissionsCommandTest.php

@@ -2,6 +2,7 @@
 
 namespace Tests\Commands;
 
+use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Bookshelf;
 use Tests\TestCase;
 
@@ -61,4 +62,21 @@ class CopyShelfPermissionsCommandTest extends TestCase
             'view' => true, 'update' => true, 'create' => false, 'delete' => false,
         ]);
     }
+
+    public function test_copy_shelf_permissions_command_using_slug_without_interaction()
+    {
+        $shelf = $this->entities->shelfHasBooks();
+        $editorRole = $this->users->editor()->roles()->first();
+        /** @var Book $child */
+        $child = $shelf->books()->first();
+        $child->shelves()->where('id', '!=', $shelf->id)->delete();
+
+        $this->assertFalse($child->hasPermissions());
+
+        $this->permissions->setEntityPermissions($shelf, ['view', 'update'], [$editorRole]);
+        $this->artisan('bookstack:copy-shelf-permissions --all --no-interaction');
+
+        $child->refresh();
+        $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
+    }
 }

tests/Commands/InstallModuleCommandTest.php

@@ -96,18 +96,44 @@ class InstallModuleCommandTest extends TestCase
         });
     }
 
-    public function test_remote_module_install_does_not_follow_redirects_to_different_origin()
+    public function test_remote_module_install_prompts_on_following_redirects_to_different_origin()
     {
         $this->usingThemeFolder(function () {
             $zip = $this->getModuleZipPath();
 
             $http = $this->mockHttpClient([
                 new Response(302, ['Location' => 'http://example.com/a-test-module.zip']),
+                new Response(301, ['Location' => 'https://a.example.com:8080/a-test-module.zip']),
                 new Response(200, ['Content-Length' => filesize($zip)], file_get_contents($zip))
             ]);
 
             $this->artisan('bookstack:install-module', ['location' => 'https://example.com/test-module.zip'])
                 ->expectsConfirmation('Are you sure you trust this source?', 'yes')
+                ->expectsOutput('The download URL is redirecting to a different site: http://example.com')
+                ->expectsConfirmation('Do you trust downloading the module from this site?', 'yes')
+                ->expectsOutput('The download URL is redirecting to a different site: https://a.example.com:8080')
+                ->expectsConfirmation('Do you trust downloading the module from this site?', 'yes')
+                ->assertExitCode(0);
+
+            $this->assertEquals(3, $http->requestCount());
+            $this->assertEquals('https', $http->requestAt(0)->getUri()->getScheme());
+            $this->assertEquals('http', $http->requestAt(1)->getUri()->getScheme());
+            $this->assertEquals('a.example.com', $http->requestAt(2)->getUri()->getHost());
+        });
+    }
+
+    public function test_remote_module_install_redirect_origin_prompt_rejection()
+    {
+        $this->usingThemeFolder(function () {
+            $http = $this->mockHttpClient([
+                new Response(302, ['Location' => 'http://example.com/a-test-module.zip']),
+                new Response(301, ['Location' => 'https://a.example.com:8080/a-test-module.zip']),
+            ]);
+
+            $this->artisan('bookstack:install-module', ['location' => 'https://example.com/test-module.zip'])
+                ->expectsConfirmation('Are you sure you trust this source?', 'yes')
+                ->expectsOutput('The download URL is redirecting to a different site: http://example.com')
+                ->expectsConfirmation('Do you trust downloading the module from this site?', 'no')
                 ->assertExitCode(1);
 
             $this->assertEquals(1, $http->requestCount());
@@ -115,6 +141,26 @@ class InstallModuleCommandTest extends TestCase
         });
     }
 
+    public function test_remote_module_install_has_redirect_limit()
+    {
+        $this->usingThemeFolder(function () {
+            $http = $this->mockHttpClient([
+                new Response(302, ['Location' => 'https://example.com/a-test-module.zip']),
+                new Response(302, ['Location' => 'https://example.com/b-test-module.zip']),
+                new Response(302, ['Location' => 'https://example.com/c-test-module.zip']),
+                new Response(302, ['Location' => 'https://example.com/d-test-module.zip']),
+            ]);
+
+            $this->artisan('bookstack:install-module', ['location' => 'https://example.com/test-module.zip'])
+                ->expectsConfirmation('Are you sure you trust this source?', 'yes')
+                ->expectsOutput('ERROR: Failed to download module from https://example.com/test-module.zip')
+                ->assertExitCode(1);
+
+            $this->assertEquals(4, $http->requestCount());
+            $this->assertEquals('/c-test-module.zip', $http->requestAt(3)->getUri()->getPath());
+        });
+    }
+
     public function test_remote_module_install_download_failures_are_announced_to_user()
     {
         $this->usingThemeFolder(function () {
@@ -175,6 +221,52 @@ class InstallModuleCommandTest extends TestCase
             ->assertExitCode(1);
     }
 
+    public function test_module_zip_when_files_in_nested_directory()
+    {
+        $this->usingThemeFolder(function ($themeFolder) {
+            $zip = new ZipArchive();
+            $zipFile = tempnam(sys_get_temp_dir(), 'bs-test-module');
+            $zip->open($zipFile, ZipArchive::CREATE);
+
+            $zip->addEmptyDir('mod');
+            $zip->addFromString('mod/bookstack-module.json', json_encode($metadata ?? [
+                'name' => 'Test Module',
+                'description' => 'A test module for BookStack',
+                'version' => '1.0.0',
+            ]));
+            $zip->addFromString('mod/functions.php', '<?php $a = "cat";');
+            $zip->addEmptyDir('mod/a');
+            $zip->addFromString('mod/a/cat.txt', 'Meow');
+            $zip->close();
+
+            $this->artisan('bookstack:install-module', ['location' => $zipFile])
+                ->expectsConfirmation('Are you sure you want to install this module?', 'yes')
+                ->assertExitCode(0);
+
+            $modulePath = glob(theme_path('modules/*'), GLOB_ONLYDIR)[0];
+            $this->assertFileExists($modulePath . '/a/cat.txt');
+            $contents = file_get_contents($modulePath . '/a/cat.txt');
+            $this->assertEquals('Meow', $contents);
+        });
+    }
+
+    public function test_module_install_negates_zip_slip()
+    {
+        $this->usingThemeFolder(function () {
+            $zip = $this->getModuleZipPath(null, [
+                '../parent.txt' => str_repeat('dog', 10)
+            ]);
+
+            $expectedInstallPath = theme_path('modules/test-module');
+            $this->artisan('bookstack:install-module', ['location' => $zip])
+                ->expectsConfirmation('Are you sure you want to install this module?', 'yes')
+                ->expectsOutput("ERROR: Failed to install module with error: Failed to load extract files from module ZIP with error: Bad file path found in module ZIP file: ../parent.txt")
+                ->assertExitCode(1);
+
+            $this->assertDirectoryDoesNotExist($expectedInstallPath);
+        });
+    }
+
     public function test_local_module_install_without_active_theme_can_setup_theme_folder()
     {
         $zip = $this->getModuleZipPath();

tests/Sorting/BookSortTest.php

@@ -271,6 +271,21 @@ class BookSortTest extends TestCase
         $this->withHtml($resp)->assertElementExists('select[name="auto-sort"] option[value="' . $sort->id . '"]');
     }
 
+    public function test_auto_sort_rule_create_hint_shown_on_sort_page()
+    {
+        $book = $this->entities->book();
+        $hintText = 'Auto sort option rules can be created in the "Lists & Sorting" settings area by a user with the relevant permissions.';
+
+        // Admin users see link for creating new rule
+        $resp = $this->asAdmin()->get($book->getUrl('/sort'));
+        $this->withHtml($resp)->assertLinkExists(url('/settings/sorting/rules/new'), 'Create Sort Rule');
+        $resp->assertDontSee($hintText);
+
+        // Non-admin users see help text
+        $resp = $this->asEditor()->get($book->getUrl('/sort'));
+        $resp->assertSee($hintText);
+    }
+
     public function test_auto_sort_option_submit_saves_to_book()
     {
         $sort = SortRule::factory()->create();

tsconfig.json

@@ -2,15 +2,16 @@
   "include": ["resources/js/**/*"],
   "exclude": ["resources/js/wysiwyg/lexical/yjs/*"],
   "compilerOptions": {
-    "target": "es2022",
-    "module": "commonjs",
+    "target": "es2023",
+    "module": "esnext",
+    "moduleResolution": "bundler",
     "rootDir": "./resources/js/",
-    "baseUrl": "./",
+    "types": ["jest", "node"],
     "paths": {
-      "@icons/*": ["resources/icons/*"],
-      "lexical": ["resources/js/wysiwyg/lexical/core/index.ts"],
-      "lexical/*": ["resources/js/wysiwyg/lexical/core/*"],
-      "@lexical/*": ["resources/js/wysiwyg/lexical/*"]
+      "@icons/*": ["./resources/icons/*"],
+      "lexical": ["./resources/js/wysiwyg/lexical/core/index.ts"],
+      "lexical/*": ["./resources/js/wysiwyg/lexical/core/*"],
+      "@lexical/*": ["./resources/js/wysiwyg/lexical/*"]
     },
     "resolveJsonModule": true,
     "allowJs": true,