fix: multi-server hardening — code correctness, security fixes, and expanded test coverage

.github/pull_request_template.md

@@ -0,0 +1,49 @@
+## Summary
+Briefly describe what this PR does in plain English, and provide as much of the following information as possible.
+
+## Motivation / Problem
+What issue does this solve?
+- Bug
+- Feature
+- Refactor
+- Maintenance / cleanup
+
+Link any related issues if applicable.
+
+## Scope of Change
+Delete items that don’t apply:
+
+- Sync/Import logic
+- NetBox models / ORM
+- LibreNMS API interaction
+- Config / settings
+- Web UI / templates
+- Database migrations
+- Tests
+- Docs only
+- Other: <describe>
+
+## How Was This Tested?
+Delete items that don’t apply and describe briefly.
+
+- Unit tests: <yes/no + what>
+- Manual testing: <yes/no + what>
+- Not tested: <explain why>
+
+### Manual Test Steps (if applicable)
+1.
+2.
+3.
+
+## Risk Assessment
+- Does this change affect existing users?  
+- Could this cause unintended imports / updates?
+
+Explain briefly.
+
+## Backwards Compatibility
+- No breaking changes
+- Breaking change (explain and document)
+
+## Other Notes
+Anything the maintainer(s) should pay particular attention to?

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.15.4  # Use the latest version from https://github.com/astral-sh/ruff-pre-commit/releases
+    rev: v0.15.5  # Use the latest version from https://github.com/astral-sh/ruff-pre-commit/releases
     hooks:
       # Run the linter
       - id: ruff-check
@@ -15,5 +15,9 @@ repos:
       - id: end-of-file-fixer
       - id: check-yaml
         exclude: ^mkdocs\.yml$
+      - id: check-yaml
+        name: check-yaml (mkdocs.yml --unsafe)
+        args: [--unsafe]
+        files: ^mkdocs\.yml$
       - id: check-added-large-files
       - id: check-merge-conflict

docs/development/testing.md

@@ -34,8 +34,18 @@ The test suite covers all major plugin functionality. Tests are organized by the
 | [test_sync_devices.py](../../netbox_librenms_plugin/tests/test_sync_devices.py) | Device sync views—field updates, platform creation, server mapping, legacy ID conversion |
 | [test_sync_interfaces.py](../../netbox_librenms_plugin/tests/test_sync_interfaces.py) | Interface sync—port matching, attribute updates, MAC handling, librenms_id assignment |
 | [test_sync_view_mismatch.py](../../netbox_librenms_plugin/tests/test_sync_view_mismatch.py) | Sync page context—device type mismatch detection and badge rendering |
+| [test_coverage_device_fields.py](../../netbox_librenms_plugin/tests/test_coverage_device_fields.py) | Device field sync view—field update logic and device field mapping |
+| [test_coverage_list.py](../../netbox_librenms_plugin/tests/test_coverage_list.py) | Import list view—background job decision, job result loading, and GET handler |
+| [test_coverage_api.py](../../netbox_librenms_plugin/tests/test_coverage_api.py) | LibreNMS API client—malformed payload guards, error paths, and edge cases |
+| [test_coverage_sync_view.py](../../netbox_librenms_plugin/tests/test_coverage_sync_view.py) | Sync view base class—context preparation and tab rendering |
+| [test_coverage_filters.py](../../netbox_librenms_plugin/tests/test_coverage_filters.py) | Import filter logic—filter form processing and device count helpers |
+| [test_sync_modules.py](../../netbox_librenms_plugin/tests/test_sync_modules.py) | Module sync—inventory matching, module type resolution, and normalization rules |
+| [test_modules_view.py](../../netbox_librenms_plugin/tests/test_modules_view.py) | Module sync view—context preparation, table rendering, and module bay mapping |
+| [test_tables_modules.py](../../netbox_librenms_plugin/tests/test_tables_modules.py) | Module tables—column rendering, row formatting, and action buttons |
 | [test_permissions.py](../../netbox_librenms_plugin/tests/test_permissions.py) | Permission enforcement—mixin contracts, object-level permissions, and write guards |
+| [test_vm_operations.py](../../netbox_librenms_plugin/tests/test_vm_operations.py) | VM operations—virtual machine sync, interface handling, and VM-specific views |
 | [test_integration_sync.py](../../netbox_librenms_plugin/tests/test_integration_sync.py) | Integration tests—API client against local mock HTTP server |
+| [test_integration_virtual_chassis.py](../../netbox_librenms_plugin/tests/test_integration_virtual_chassis.py) | Integration tests—VC detection, negative cache, multi-server cache isolation |
 | [test_view_wiring.py](../../netbox_librenms_plugin/tests/test_view_wiring.py) | Smoke tests—view class MRO, mixin wiring, permission contracts, and template syntax |
 
 Supporting files:
@@ -76,8 +86,11 @@ pytest netbox_librenms_plugin/tests/test_background_jobs.py -v
 # Multi-server librenms_id tests
 pytest netbox_librenms_plugin/tests/test_librenms_id.py -v
 
-# Sync view tests (devices, interfaces)
-pytest netbox_librenms_plugin/tests/test_sync_devices.py netbox_librenms_plugin/tests/test_sync_interfaces.py -v
+# Sync view tests (devices, interfaces, modules)
+pytest netbox_librenms_plugin/tests/test_sync_devices.py netbox_librenms_plugin/tests/test_sync_interfaces.py netbox_librenms_plugin/tests/test_sync_modules.py -v
+
+# Integration tests (API client against mock HTTP server)
+pytest netbox_librenms_plugin/tests/test_integration_sync.py -v
 
 # Sync view mismatch detection and permission enforcement
 pytest netbox_librenms_plugin/tests/test_sync_view_mismatch.py netbox_librenms_plugin/tests/test_permissions.py -v
@@ -110,9 +123,10 @@ pytest netbox_librenms_plugin/tests/ -v --lf
 The test suite prioritizes speed and isolation so you can run tests frequently during development:
 
 - **Mock-based**: Unit tests use `MagicMock` instead of real database objects. No Django database setup required.
-- **Fast execution**: The full suite runs in under 0.5 seconds.
+- **Fast execution**: The full suite runs in approximately 15-20 seconds (varies by environment).
 - **Isolated**: Each test is independent with no shared state between tests.
 - **No external network access**: Tests never call external services. Integration tests use a local loopback HTTP server (`mock_librenms_server.py`) to exercise the real API client against realistic HTTP responses without requiring a running LibreNMS instance.
+- **Coverage exclusions**: Test files themselves are excluded from coverage reports (see `[tool.coverage.run]` omit list in `pyproject.toml`).
 
 This approach means tests work identically in your local development environment, in the devcontainer, and in CI pipelines.
 

netbox_librenms_plugin/forms.py

@@ -60,8 +60,8 @@ def _get_librenms_poller_group_choices():
         api = LibreNMSAPI()
         success, poller_groups = api.get_poller_groups()
 
-        if success and poller_groups:
-            for group in poller_groups:
+        if success:
+            for group in poller_groups or []:
                 group_id = str(group.get("id", ""))
                 group_name = group.get("group_name", "")
                 group_descr = group.get("descr", "")
@@ -581,9 +581,26 @@ def _populate_librenms_locations(self):
         from netbox_librenms_plugin.librenms_api import LibreNMSAPI
 
         try:
-            # Instantiate the API client to resolve the authoritative server_key
+            # Determine server_key cheaply from settings to check cache before instantiating the API
+            try:
+                from netbox_librenms_plugin.models import LibreNMSSettings
+
+                _settings = LibreNMSSettings.objects.first()
+                _server_key = (_settings.selected_server if _settings else None) or "default"
+            except Exception:
+                _server_key = "default"
+
+            cache_key = get_location_choices_cache_key(_server_key)
+            cached_choices = cache.get(cache_key)
+            if cached_choices:
+                self.fields["librenms_location"].choices = cached_choices
+                return
+
+            # Cache miss — instantiate the API client and fetch
             api = LibreNMSAPI()
+            # Recompute cache_key with the resolved server_key in case it differs from settings
             cache_key = get_location_choices_cache_key(api.server_key)
+            # Second cache check: the resolved server_key may differ from the settings key
             cached_choices = cache.get(cache_key)
             if cached_choices:
                 self.fields["librenms_location"].choices = cached_choices

netbox_librenms_plugin/import_utils/__init__.py

@@ -23,6 +23,7 @@
     get_active_cached_searches,
     get_cache_metadata_key,
     get_import_device_cache_key,
+    get_import_search_cache_key,
     get_validated_device_cache_key,
 )
 from .device_operations import (  # noqa: F401