From f2f76a3c5658456a6d1d72174defed44c7f520cb Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 6 Mar 2026 09:28:46 +0000 Subject: [PATCH] Modules: Improved install command based on testing - Updated output to be clearer - Added warning and confirmation to local install flow - Adjusted module folder name creation --- app/Console/Commands/InstallModuleCommand.php | 11 +++++++++-- app/Theming/ThemeModuleManager.php | 2 +- tests/Commands/InstallModuleCommandTest.php | 14 ++++++++++++-- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/Console/Commands/InstallModuleCommand.php b/app/Console/Commands/InstallModuleCommand.php index dc3c9363e..20252525d 100644 --- a/app/Console/Commands/InstallModuleCommand.php +++ b/app/Console/Commands/InstallModuleCommand.php @@ -268,7 +268,7 @@ class InstallModuleCommand extends Command if ($isRemote) { // Warning about fetching from source $host = parse_url($location, PHP_URL_HOST); - $this->warn("This will download a module from {$host}. Modules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources."); + $this->warn("\nThis will download a module from: {$host}\n\nModules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources."); $trustHost = $this->confirm('Are you sure you trust this source?'); if (!$trustHost) { return null; @@ -286,13 +286,20 @@ class InstallModuleCommand extends Command return $this->downloadModuleFile($location); } - // Validate file and get full location + // Validate the file and get the full location $zipPath = realpath($location); + if (!$zipPath || !is_file($zipPath)) { $this->error("ERROR: Module file not found at {$location}"); return null; } + $this->warn("\nThis will install a module from: {$zipPath}\n\nModules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources."); + $trustHost = $this->confirm('Are you sure you want to install this module?'); + if (!$trustHost) { + return null; + } + return $zipPath; } diff --git a/app/Theming/ThemeModuleManager.php b/app/Theming/ThemeModuleManager.php index 900063d47..86362a2f3 100644 --- a/app/Theming/ThemeModuleManager.php +++ b/app/Theming/ThemeModuleManager.php @@ -44,7 +44,7 @@ class ThemeModuleManager */ public function addFromZip(string $name, ThemeModuleZip $zip): ThemeModule { - $baseFolderName = Str::limit(Str::slug($name), 20); + $baseFolderName = Str::limit(Str::slug($name), 40, ''); $folderName = $baseFolderName; while (!$baseFolderName || file_exists($this->modulesFolderPath . DIRECTORY_SEPARATOR . $folderName)) { $folderName = ($baseFolderName ?: 'mod') . '-' . Str::random(4); diff --git a/tests/Commands/InstallModuleCommandTest.php b/tests/Commands/InstallModuleCommandTest.php index 0872efc3f..8ffc4ead3 100644 --- a/tests/Commands/InstallModuleCommandTest.php +++ b/tests/Commands/InstallModuleCommandTest.php @@ -15,6 +15,8 @@ class InstallModuleCommandTest extends TestCase $zip = $this->getModuleZipPath(); $expectedInstallPath = theme_path('modules/test-module'); $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsOutput("\nThis will install a module from: {$zip}\n\nModules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources.") + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput('Module "Test Module" (v1.0.0) successfully installed!') ->expectsOutput("Install location: {$expectedInstallPath}") ->assertExitCode(0); @@ -35,7 +37,7 @@ class InstallModuleCommandTest extends TestCase $expectedInstallPath = theme_path('modules/test-module'); $this->artisan('bookstack:install-module', ['location' => 'https://example.com/test-module.zip']) - ->expectsOutput("This will download a module from example.com. Modules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources.") + ->expectsOutput("\nThis will download a module from: example.com\n\nModules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources.") ->expectsConfirmation('Are you sure you trust this source?', 'yes') ->expectsOutput('Module "Test Module" (v1.0.0) successfully installed!') ->expectsOutput("Install location: {$expectedInstallPath}") @@ -61,7 +63,7 @@ class InstallModuleCommandTest extends TestCase $expectedInstallPath = theme_path('modules/test-module'); $this->artisan('bookstack:install-module', ['location' => 'http://example.com/test-module.zip']) - ->expectsOutput("This will download a module from example.com. Modules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources.") + ->expectsOutput("\nThis will download a module from: example.com\n\nModules can contain code which would have the ability to do anything on the BookStack host server.\nYou should only install modules from trusted sources.") ->expectsConfirmation('Are you sure you trust this source?', 'yes') ->expectsOutput("You are downloading a module from an insecure HTTP source.\nWe recommend only using HTTPS sources to avoid various security risks.") ->expectsConfirmation('Are you sure you want to continue without HTTPS?', 'yes') @@ -142,6 +144,7 @@ class InstallModuleCommandTest extends TestCase file_put_contents($zip, 'invalid zip'); $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput("ERROR: Cannot open ZIP file at {$zip}") ->assertExitCode(1); } @@ -153,6 +156,7 @@ class InstallModuleCommandTest extends TestCase ]); $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput("ERROR: Module ZIP file contents are too large. Maximum size is 50MB") ->assertExitCode(1); } @@ -166,6 +170,7 @@ class InstallModuleCommandTest extends TestCase ]); $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput("ERROR: Failed to read module metadata with error: Module in folder \"_temp\" has an invalid 'version' format. Expected semantic version format like '1.0.0' or 'v1.0.0'") ->assertExitCode(1); } @@ -177,6 +182,7 @@ class InstallModuleCommandTest extends TestCase File::deleteDirectory($expectedThemePath); $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsConfirmation('No active theme folder found, would you like to create one?', 'yes') ->expectsOutput("Created theme folder at {$expectedThemePath}") ->expectsOutput("You will need to set APP_THEME=custom in your BookStack env configuration to enable this theme!") @@ -195,6 +201,7 @@ class InstallModuleCommandTest extends TestCase File::put(theme_path('modules'), '{}'); $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput("ERROR: Cannot create a modules folder, file already exists at " . theme_path('modules')) ->assertExitCode(1); }); @@ -207,6 +214,7 @@ class InstallModuleCommandTest extends TestCase $new = $this->getModuleZipPath(['name' => 'Test Module', 'description' => '', 'version' => '2.0.0']); $this->artisan('bookstack:install-module', ['location' => $new]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput('The following modules already exist with the same name:') ->expectsOutput('Test Module (test-module:v1.0.0) - cat') ->expectsChoice('What would you like to do?', 'Replace existing module', ['Cancel module install', 'Add alongside existing module', 'Replace existing module']) @@ -226,6 +234,7 @@ class InstallModuleCommandTest extends TestCase $new = $this->getModuleZipPath(['name' => 'Test Module', 'description' => '', 'version' => '2.0.0']); $this->artisan('bookstack:install-module', ['location' => $new]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput('The following modules already exist with the same name:') ->expectsOutput('Test Module (test-module:v1.0.0) - cat') ->expectsChoice('What would you like to do?', 'Cancel module install', ['Cancel module install', 'Add alongside existing module', 'Replace existing module']) @@ -244,6 +253,7 @@ class InstallModuleCommandTest extends TestCase $new = $this->getModuleZipPath(['name' => 'Test Module', 'description' => '', 'version' => '2.0.0']); $this->artisan('bookstack:install-module', ['location' => $new]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') ->expectsOutput('The following modules already exist with the same name:') ->expectsOutput('Test Module (test-module:v1.0.0) - cat') ->expectsChoice('What would you like to do?', 'Add alongside existing module', ['Cancel module install', 'Add alongside existing module', 'Replace existing module'])