fix(plugins): enforce strict validation and integrity checks for missing custom parser #1812

backend/secuscan/plugins.py

@@ -218,7 +218,11 @@ async def _validate_plugin(self, plugin: PluginMetadata, plugin_dir: Path) -> bo
         # Validate parser exists
         parser_file = plugin_dir / "parser.py"
         if plugin.output.get("parser") == "custom" and not parser_file.exists():
-            logger.warning("Custom parser specified but parser.py not found")
+            logger.error(
+                "Plugin %s declares output.parser == 'custom' but parser.py "
+                "is missing from %s", plugin.id, plugin_dir,
+            )
+            return False
 
         if not self._validate_safety(plugin.safety):
             return False
@@ -242,8 +246,11 @@ def _verify_plugin_integrity(self, plugin: PluginMetadata, plugin_dir: Path) ->
             logger.error("Plugin %s missing checksum/signature while enforcement is enabled", plugin.id)
             return False
 
+        requires_parser = plugin.output.get("parser") == "custom"
         try:
-            combined_digest = self.compute_plugin_digest(metadata_file, parser_file)
+            combined_digest = self.compute_plugin_digest(
+                metadata_file, parser_file, require_parser=requires_parser
+            )
         except Exception as exc:
             logger.error("Failed to hash plugin files for %s: %s", plugin.id, exc)
             return False
@@ -269,14 +276,31 @@ def _verify_plugin_integrity(self, plugin: PluginMetadata, plugin_dir: Path) ->
         return True
 
     @staticmethod
-    def compute_plugin_digest(metadata_file: Path, parser_file: Path) -> str:
-        """Compute deterministic plugin digest ignoring mutable checksum/signature fields."""
+    def compute_plugin_digest(
+        metadata_file: Path, parser_file: Path, require_parser: bool = False
+    ) -> str:
+        """Compute deterministic plugin digest ignoring mutable checksum/signature fields.
+
+        Args:
+            require_parser: When True, parser_file must exist. Used for plugins
+                that declare output.parser == 'custom', where a missing parser.py
+                would otherwise be silently hashed as an empty digest and let a
+                broken plugin pass integrity verification.
+
+        Raises:
+            FileNotFoundError: If require_parser is True and parser_file is missing.
+        """
         metadata = json.loads(metadata_file.read_text(encoding="utf-8"))
         metadata.pop("checksum", None)
         metadata.pop("signature", None)
         metadata_canonical = json.dumps(metadata, sort_keys=True, separators=(",", ":"))
         metadata_digest = hashlib.sha256(metadata_canonical.encode("utf-8")).hexdigest()
 
+        if require_parser and not parser_file.exists():
+            raise FileNotFoundError(
+                f"Custom parser required but not found: {parser_file}"
+            )
+
         parser_digest = ""
         if parser_file.exists():
             parser_bytes = parser_file.read_bytes()

testing/backend/unit/test_plugins_validation.py

@@ -0,0 +1,240 @@
+"""
+Tests for issue #1812 — enforce hard failure when a plugin declares
+output.parser == 'custom' but parser.py is missing from the plugin's
+directory.
+
+What we're testing:
+- _validate_plugin rejects (returns False) a custom-parser plugin missing
+  parser.py, instead of the old soft-warning-and-pass behavior.
+- compute_plugin_digest(..., require_parser=True) raises FileNotFoundError
+  when parser.py is absent.
+- _verify_plugin_integrity propagates that failure for custom-parser plugins.
+- Plugins using non-custom (built-in) parsers never require parser.py and
+  continue to load/verify successfully — no regression for the common case.
+"""
+
+import asyncio
+import json
+from pathlib import Path
+
+import pytest
+
+from backend.secuscan.plugins import PluginManager
+
+
+def make_plugin(
+    tmp_path: Path,
+    plugin_id: str,
+    *,
+    checksum: str | None = "auto",
+    parser_type: str = "custom",
+    parser_content: str | None = "def parse(output): return []",
+    include_name: bool = True,
+) -> Path:
+    """Build a minimal, well-formed plugin directory under tmp_path."""
+    plugin_dir = tmp_path / plugin_id
+    plugin_dir.mkdir()
+
+    metadata = {
+        "id": plugin_id,
+        "version": "1.0.0",
+        "description": "Test plugin",
+        "category": "recon",
+        "engine": {"type": "cli", "binary": "echo"},
+        "command_template": ["echo", "{target}"],
+        "fields": [
+            {"id": "target", "label": "Target", "type": "string", "required": True}
+        ],
+        "presets": {},
+        "output": {"format": "text", "parser": parser_type},
+        "safety": {"level": "safe"},
+    }
+
+    if include_name:
+        metadata["name"] = f"Test Plugin {plugin_id}"
+
+    if parser_content is not None:
+        (plugin_dir / "parser.py").write_text(parser_content, encoding="utf-8")
+
+    metadata_file = plugin_dir / "metadata.json"
+    metadata_file.write_text(json.dumps(metadata, indent=2), encoding="utf-8")
+
+    if checksum == "auto":
+        expected = PluginManager.compute_plugin_digest(metadata_file, plugin_dir / "parser.py")
+        metadata["checksum"] = expected
+    elif checksum is not None:
+        metadata["checksum"] = checksum
+
+    metadata_file.write_text(json.dumps(metadata, indent=2) + "\n", encoding="utf-8")
+    return plugin_dir
+
+
+# ---------------------------------------------------------------------------
+# 1. Happy path — custom parser present, everything passes
+# ---------------------------------------------------------------------------
+
+
+def test_custom_parser_present_passes_validation(tmp_path):
+    manager = PluginManager(str(tmp_path))
+    plugin_dir = make_plugin(tmp_path, "custom-ok")
+    metadata_file = plugin_dir / "metadata.json"
+
+    plugin_meta = asyncio.run(manager._load_plugin_metadata(metadata_file))
+
+    assert asyncio.run(manager._validate_plugin(plugin_meta, plugin_dir)) is True
+
+
+def test_custom_parser_present_plugin_loads_via_load_plugins(tmp_path):
+    make_plugin(tmp_path, "custom-ok")
+    manager = PluginManager(str(tmp_path))
+
+    loaded = asyncio.run(manager.load_plugins())
+
+    assert loaded == 1
+    assert manager.get_plugin("custom-ok") is not None
+
+
+# ---------------------------------------------------------------------------
+# 2. Failure path — _validate_plugin hard-fails when parser.py missing
+# ---------------------------------------------------------------------------
+
+
+def test_validate_plugin_fails_when_custom_parser_missing(tmp_path):
+    manager = PluginManager(str(tmp_path))
+    plugin_dir = make_plugin(tmp_path, "custom-missing", parser_content=None, checksum=None)
+    metadata_file = plugin_dir / "metadata.json"
+
+    plugin_meta = asyncio.run(manager._load_plugin_metadata(metadata_file))
+
+    assert asyncio.run(manager._validate_plugin(plugin_meta, plugin_dir)) is False
+
+
+def test_validate_plugin_logs_error_not_warning_when_parser_missing(tmp_path, caplog):
+    manager = PluginManager(str(tmp_path))
+    plugin_dir = make_plugin(tmp_path, "custom-missing-log", parser_content=None, checksum=None)
+    metadata_file = plugin_dir / "metadata.json"
+    plugin_meta = asyncio.run(manager._load_plugin_metadata(metadata_file))
+
+    with caplog.at_level("ERROR", logger="backend.secuscan.plugins"):
+        result = asyncio.run(manager._validate_plugin(plugin_meta, plugin_dir))
+
+    assert result is False
+    assert any(
+        "custom" in record.message and "parser.py" in record.message
+        for record in caplog.records
+        if record.levelname == "ERROR"
+    )
+
+
+def test_load_plugins_skips_plugin_with_missing_custom_parser(tmp_path):
+    """End-to-end: load_plugins() must not register a broken custom-parser plugin."""
+    make_plugin(tmp_path, "custom-missing-e2e", parser_content=None, checksum=None)
+    manager = PluginManager(str(tmp_path))
+
+    loaded = asyncio.run(manager.load_plugins())
+
+    assert loaded == 0
+    assert manager.get_plugin("custom-missing-e2e") is None
+
+
+# ---------------------------------------------------------------------------
+# 3. compute_plugin_digest(require_parser=True) raises FileNotFoundError
+# ---------------------------------------------------------------------------
+
+
+def test_compute_plugin_digest_raises_when_parser_required_but_missing(tmp_path):
+    plugin_dir = make_plugin(tmp_path, "digest-missing", parser_content=None, checksum=None)
+    metadata_file = plugin_dir / "metadata.json"
+    parser_file = plugin_dir / "parser.py"
+
+    assert not parser_file.exists()
+
+    with pytest.raises(FileNotFoundError):
+        PluginManager.compute_plugin_digest(metadata_file, parser_file, require_parser=True)
+
+
+def test_compute_plugin_digest_succeeds_when_parser_required_and_present(tmp_path):
+    plugin_dir = make_plugin(tmp_path, "digest-present", checksum=None)
+    metadata_file = plugin_dir / "metadata.json"
+    parser_file = plugin_dir / "parser.py"
+
+    digest = PluginManager.compute_plugin_digest(metadata_file, parser_file, require_parser=True)
+
+    assert isinstance(digest, str)
+    assert len(digest) == 64  # sha256 hex digest
+
+
+def test_compute_plugin_digest_default_does_not_require_parser(tmp_path):
+    """Backward-compat: require_parser defaults to False, so existing call
+    sites that never pass it keep the old lenient (empty parser digest) behavior.
+    """
+    plugin_dir = make_plugin(tmp_path, "digest-legacy", parser_content=None, checksum=None)
+    metadata_file = plugin_dir / "metadata.json"
+    parser_file = plugin_dir / "parser.py"
+
+    digest = PluginManager.compute_plugin_digest(metadata_file, parser_file)
+
+    assert isinstance(digest, str)
+    assert len(digest) == 64
+
+
+def test_verify_plugin_integrity_fails_for_custom_parser_missing_at_verify_time(tmp_path):
+    """_verify_plugin_integrity must pass require_parser=True for custom-parser
+    plugins so a parser.py deleted after metadata load is still caught.
+    """
+    manager = PluginManager(str(tmp_path))
+    plugin_dir = make_plugin(tmp_path, "verify-missing")
+    metadata_file = plugin_dir / "metadata.json"
+    plugin_meta = asyncio.run(manager._load_plugin_metadata(metadata_file))
+
+    # Parser existed at checksum-computation time, but is now deleted —
+    # simulates a race / tamper between metadata load and integrity check.
+    (plugin_dir / "parser.py").unlink()
+
+    assert manager._verify_plugin_integrity(plugin_meta, plugin_dir) is False
+
+
+# ---------------------------------------------------------------------------
+# 4. Backward compatibility — non-custom parsers never require parser.py
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize("parser_type", ["default", "json", "regex", "builtin"])
+def test_non_custom_parser_does_not_require_parser_file(tmp_path, parser_type):
+    manager = PluginManager(str(tmp_path))
+    plugin_dir = make_plugin(
+        tmp_path, f"builtin-{parser_type}", parser_type=parser_type, parser_content=None, checksum=None
+    )
+    metadata_file = plugin_dir / "metadata.json"
+    plugin_meta = asyncio.run(manager._load_plugin_metadata(metadata_file))
+
+    assert not (plugin_dir / "parser.py").exists()
+    assert asyncio.run(manager._validate_plugin(plugin_meta, plugin_dir)) is True
+
+
+def test_non_custom_parser_plugin_loads_via_load_plugins(tmp_path):
+    make_plugin(tmp_path, "builtin-ok", parser_type="default", parser_content=None, checksum=None)
+    manager = PluginManager(str(tmp_path))
+
+    loaded = asyncio.run(manager.load_plugins())
+
+    assert loaded == 1
+    assert manager.get_plugin("builtin-ok") is not None
+
+
+def test_compute_plugin_digest_require_parser_false_for_builtin_parser(tmp_path):
+    """Digest computation for a non-custom plugin must not raise even though
+    require_parser is being decided from plugin.output.parser upstream —
+    here we assert the low-level contract directly: require_parser=False
+    tolerates a missing parser.py.
+    """
+    plugin_dir = make_plugin(
+        tmp_path, "builtin-digest", parser_type="default", parser_content=None, checksum=None
+    )
+    metadata_file = plugin_dir / "metadata.json"
+    parser_file = plugin_dir / "parser.py"
+
+    digest = PluginManager.compute_plugin_digest(metadata_file, parser_file, require_parser=False)
+
+    assert isinstance(digest, str)
+    assert len(digest) == 64

testing/backend/unit/test_tls_verification.py

@@ -51,18 +51,47 @@ def test_env_override_true(self, monkeypatch):
 # ---------------------------------------------------------------------------
 
 
+class _MockStreamContextManager:
+    """Mimics httpx's ``client.stream(...)`` return value.
+
+    ``AsyncClient.stream()`` is a *synchronous* call that returns an async
+    context manager (it is not itself a coroutine) — the response is only
+    produced on ``__aenter__``. A plain ``AsyncMock`` attribute would make
+    ``client.stream(...)`` return a coroutine instead, which blows up with
+    "'coroutine' object does not support the asynchronous context manager
+    protocol" the moment ``crawl_target`` does ``async with client.stream(...)``.
+    """
+
+    def __init__(self, response):
+        self._response = response
+
+    async def __aenter__(self):
+        return self._response
+
+    async def __aexit__(self, exc_type, exc, tb):
+        return False
+
+
+def _make_mock_stream_response(body: bytes = b"<html></html>", url: str = "https://example.com/"):
+    async def _aiter_bytes():
+        yield body
+
+    mock_response = MagicMock()
+    mock_response.url = url
+    mock_response.status_code = 200
+    mock_response.headers = {}
+    mock_response.history = []
+    mock_response.aiter_bytes = MagicMock(return_value=_aiter_bytes())
+    return mock_response
+
+
 class TestCrawlerVerifySsl:
     @pytest.mark.asyncio
     async def test_crawl_target_passes_verify_ssl(self):
-        mock_response = MagicMock()
-        mock_response.text = "<html></html>"
-        mock_response.url = "https://example.com/"
-        mock_response.status_code = 200
-        mock_response.headers = {}
-        mock_response.history = []
+        mock_response = _make_mock_stream_response()
 
         mock_client = AsyncMock()
-        mock_client.get = AsyncMock(return_value=mock_response)
+        mock_client.stream = MagicMock(return_value=_MockStreamContextManager(mock_response))
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -79,15 +108,10 @@ async def test_crawl_target_passes_verify_ssl(self):
 
     @pytest.mark.asyncio
     async def test_crawl_target_verify_ssl_false_when_disabled(self):
-        mock_response = MagicMock()
-        mock_response.text = "<html></html>"
-        mock_response.url = "https://example.com/"
-        mock_response.status_code = 200
-        mock_response.headers = {}
-        mock_response.history = []
+        mock_response = _make_mock_stream_response()
 
         mock_client = AsyncMock()
-        mock_client.get = AsyncMock(return_value=mock_response)
+        mock_client.stream = MagicMock(return_value=_MockStreamContextManager(mock_response))
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -116,6 +140,9 @@ async def test_api_scanner_passes_verify_ssl(self):
 
         mock_client = AsyncMock()
         mock_client.get = AsyncMock(return_value=MagicMock(status_code=404, text="", headers={}))
+        mock_client.stream = MagicMock(
+            return_value=_MockStreamContextManager(_make_mock_stream_response())
+        )
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -138,6 +165,9 @@ async def test_api_scanner_verify_ssl_false_when_disabled(self):
 
         mock_client = AsyncMock()
         mock_client.get = AsyncMock(return_value=MagicMock(status_code=404, text="", headers={}))
+        mock_client.stream = MagicMock(
+            return_value=_MockStreamContextManager(_make_mock_stream_response())
+        )
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)