Merge commit from fork

* Add note to plugin docs.

* Adjust logic for PluginListTable

* Add superuser scope to PluginInstall API endpoint

* Update unit test for API endpoint

* Explicitly set PLUGINS_INSTALL_DISABLED if PLUGINS_ENABLED = False

* Check for superuser permission in installer.py

* Additional user checks

* Sanitize package name to protect against OS command injection

docs/docs/plugins/install.md

@@ -74,6 +74,9 @@ Enter the package name into the form as shown below. You can add a path and a ve
 
 {{ image("plugin/plugin_install_txt.png", "Plugin.txt file") }}
 
+!!! info "Superuser Required"
+    Only users with superuser privileges can manage plugins via the web interface.
+
 #### Local Directory
 
 Custom plugins can be placed in the `data/plugins/` directory, where they will be automatically discovered. This can be useful for developing and testing plugins, but can prove more difficult in production (e.g. when using Docker).

src/backend/InvenTree/InvenTree/settings.py

@@ -201,6 +201,11 @@ PLUGINS_INSTALL_DISABLED = get_boolean_setting(
     'INVENTREE_PLUGIN_NOINSTALL', 'plugin_noinstall', False
 )
 
+if not PLUGINS_ENABLED:
+    PLUGINS_INSTALL_DISABLED = (
+        True  # If plugins are disabled, also disable installation
+    )
+
 PLUGIN_FILE = config.get_plugin_file()
 
 # Plugin test settings

src/backend/InvenTree/plugin/api.py

@@ -210,6 +210,7 @@ class PluginInstall(CreateAPI):
 
     queryset = PluginConfig.objects.none()
     serializer_class = PluginSerializers.PluginConfigInstallSerializer
+    permission_classes = [InvenTree.permissions.IsSuperuserOrSuperScope]
 
     def create(self, request, *args, **kwargs):
         """Install a plugin via the API."""

src/backend/InvenTree/plugin/installer.py

@@ -236,8 +236,8 @@ def install_plugin(url=None, packagename=None, user=None, version=None):
         user: Optional user performing the installation
         version: Optional version specifier
     """
-    if user and not user.is_staff:
+    if user and not user.is_superuser:
-        raise ValidationError(_('Only staff users can administer plugins'))
+        raise ValidationError(_('Only superuser accounts can administer plugins'))
 
     if settings.PLUGINS_INSTALL_DISABLED:
         raise ValidationError(_('Plugin installation is disabled'))
@@ -269,6 +269,13 @@ def install_plugin(url=None, packagename=None, user=None, version=None):
         if version:
             full_pkg = f'{full_pkg}=={version}'
 
+    if not full_pkg:
+        raise ValidationError(_('No package name or URL provided for installation'))
+
+    # Sanitize the package name for installation
+    if any(c in full_pkg for c in ';&|`$()'):
+        raise ValidationError(_('Invalid characters in package name or URL'))
+
     install_name.append(full_pkg)
 
     ret = {}
@@ -333,6 +340,9 @@ def uninstall_plugin(cfg: plugin.models.PluginConfig, user=None, delete_config=T
     """
     from plugin.registry import registry
 
+    if user and not user.is_superuser:
+        raise ValidationError(_('Only superuser accounts can administer plugins'))
+
     if settings.PLUGINS_INSTALL_DISABLED:
         raise ValidationError(_('Plugin uninstalling is disabled'))
 

src/backend/InvenTree/plugin/serializers.py

@@ -165,6 +165,9 @@ class PluginConfigInstallSerializer(serializers.Serializer):
         version = data.get('version', None)
         user = self.context['request'].user
 
+        if not user or not user.is_superuser:
+            raise ValidationError(_('Only superuser accounts can administer plugins'))
+
         return install_plugin(
             url=url, packagename=packagename, version=version, user=user
         )
@@ -266,10 +269,13 @@ class PluginUninstallSerializer(serializers.Serializer):
         """Uninstall the specified plugin."""
         from plugin.installer import uninstall_plugin
 
+        user = self.context['request'].user
+
+        if not user or not user.is_superuser:
+            raise ValidationError(_('Only superuser accounts can administer plugins'))
+
         return uninstall_plugin(
-            instance,
+            instance, user=user, delete_config=validated_data.get('delete_config', True)
-            user=self.context['request'].user,
-            delete_config=validated_data.get('delete_config', True),
         )
 
 

src/backend/InvenTree/plugin/test_api.py

@@ -63,6 +63,21 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
         """Test the plugin install command."""
         url = reverse('api-plugin-install')
 
+        # Requires superuser permissions
+        self.user.is_superuser = False
+        self.user.save()
+
+        self.post(
+            url,
+            {'confirm': True, 'packagename': self.PKG_NAME},
+            expected_code=403,
+            max_query_time=30,
+        )
+
+        # Provide superuser permissions
+        self.user.is_superuser = True
+        self.user.save()
+
         # invalid package name
         data = self.post(
             url,
@@ -209,7 +224,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
         test_plg.refresh_from_db()
         self.assertTrue(test_plg.is_active())
 
-    def test_pluginCfg_delete(self):
+    def test_plugin_config_delete(self):
         """Test deleting a config."""
         test_plg = self.plugin_confs.first()
         assert test_plg is not None

src/backend/InvenTree/plugin/test_plugin.py

@@ -11,11 +11,14 @@ from typing import Optional
 from unittest import mock
 from unittest.mock import patch
 
+from django.contrib.auth.models import User
+from django.core.exceptions import ValidationError
 from django.test import TestCase, override_settings
 
 import plugin.templatetags.plugin_extras as plugin_tags
 from InvenTree.unit_test import PluginRegistryMixin, TestQueryMixin
 from plugin import InvenTreePlugin, PluginMixinEnum
+from plugin.installer import install_plugin
 from plugin.registry import registry
 from plugin.samples.integration.another_sample import (
     NoIntegrationPlugin,
@@ -326,6 +329,9 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase):
 
     def test_broken_samples(self):
         """Test that the broken samples trigger reloads."""
+        # Reset the registry to a known state
+        registry.errors = {}
+
         # In the base setup there are no errors
         self.assertEqual(len(registry.errors), 0)
 
@@ -686,3 +692,40 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase):
         self.assertTrue(cfg.is_builtin())
         self.assertFalse(cfg.is_package())
         self.assertFalse(cfg.is_sample())
+
+
+class InstallerTests(TestCase):
+    """Tests for the plugin installer code."""
+
+    def test_plugin_install_errors(self):
+        """Test error handling for plugin installation."""
+        # No data provided
+        with self.assertRaises(ValidationError) as e:
+            install_plugin()
+
+        self.assertIn(
+            'No package name or URL provided for installation', str(e.exception)
+        )
+
+        # Invalid package name
+        for pkg in [
+            'invalid;name',
+            'invalid&name',
+            'invalid|name',
+            'invalid`name',
+            'invalid$(name)',
+        ]:
+            with self.assertRaises(ValidationError) as e:
+                install_plugin(packagename=pkg)
+
+            self.assertIn('Invalid characters in package name or URL', str(e.exception))
+
+        # Non superuser account
+        user = User.objects.create(username='my-user', is_superuser=False)
+
+        with self.assertRaises(ValidationError) as e:
+            install_plugin(user=user, packagename='some-package')
+
+        self.assertIn(
+            'Only superuser accounts can administer plugins', str(e.exception)
+        )

src/frontend/src/tables/plugin/PluginListTable.tsx

@@ -220,7 +220,6 @@ export default function PluginListTable() {
           // Uninstall an installed plugin
           // Must be inactive, not a builtin, not a sample, and installed as a package
           hidden:
-            !user.isSuperuser() ||
             record.active ||
             record.is_builtin ||
             record.is_mandatory ||
@@ -244,8 +243,7 @@ export default function PluginListTable() {
             record.is_builtin ||
             record.is_mandatory ||
             record.is_sample ||
-            record.is_installed ||
+            record.is_installed,
-            !user.isSuperuser(),
           title: t`Delete`,
           tooltip: t`Delete selected plugin configuration`,
           color: 'red',
@@ -355,7 +353,12 @@ export default function PluginListTable() {
 
   // Custom table actions
   const tableActions = useMemo(() => {
-    if (!user.isSuperuser() || !server.plugins_enabled) {
+    if (
+      !user.isSuperuser() ||
+      !server.plugins_enabled ||
+      server.plugins_install_disabled
+    ) {
+      // Prevent installation if plugins are disabled or user is not superuser
       return [];
     }
 
@@ -376,7 +379,6 @@ export default function PluginListTable() {
           setPluginPackage('');
           installPluginModal.open();
         }}
-        disabled={server.plugins_install_disabled || false}
       />
     ];
   }, [user, server]);