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

Author: ssddanbrown

app/Console/Commands/InstallModuleCommand.php

@@ -268,7 +268,7 @@ protected function getPathToZip(string $location): string|null
         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 @@ protected function getPathToZip(string $location): string|null
             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;
     }
 

app/Theming/ThemeModuleManager.php

@@ -44,7 +44,7 @@ public function deleteModuleFolder(string $moduleFolderName): void
      */
     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);

tests/Commands/InstallModuleCommandTest.php

@@ -15,6 +15,8 @@ public function test_local_module_install_with_active_theme()
             $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 @@ public function test_remote_module_install_with_active_theme()
             $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 @@ public function test_remote_http_module_warns_and_prompts_users()
             $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 @@ public function test_run_with_invalid_zip_has_early_exit()
         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 @@ public function test_run_with_large_zip_has_early_exit()
         ]);
 
         $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 @@ public function test_run_with_invalid_module_data_has_early_exit()
         ]);
 
         $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 @@ public function test_local_module_install_without_active_theme_can_setup_theme_f
         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 @@ public function test_local_module_install_with_active_theme_and_conflicting_modu
             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 @@ public function test_single_existing_module_with_same_name_replace()
             $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 @@ public function test_single_existing_module_with_same_name_cancel()
             $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 @@ public function test_single_existing_module_with_same_name_add()
             $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'])