Feat/test system by AFredefon · Pull Request #27 · FuzzingLabs/mcp-security-hub

AFredefon · 2026-04-08T01:21:56Z

Refactor test system — remove manifest.yaml for full servers, keep only for wrappers

… wrappers

Copilot

Pull request overview

Refactors the hub’s test/validation system to auto-discover MCP servers, require manifest.yaml only for wrapper servers, and add an opt-in Docker-based MCP smoke test pipeline in CI.

Changes:

Added a tiered, discovery-driven pytest suite for structure + MCP compliance + Docker smoke tests.
Introduced a Docker JSON-RPC harness to validate MCP initialize + tools/list behavior inside containers.
Added wrapper manifest.yaml scaffolds and CI workflows/markers to support the new validation model.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
web-security/nikto-mcp/manifest.yaml	Adds wrapper manifest metadata for nikto wrapper server.
web-security/burp-mcp/manifest.yaml	Adds wrapper manifest metadata for burp wrapper server.
threat-intel/virustotal-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
threat-intel/otx-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
reconnaissance/zoomeye-mcp/manifest.yaml	Adds wrapper manifest metadata for zoomeye wrapper server.
reconnaissance/shodan-mcp/manifest.yaml	Adds wrapper manifest metadata for shodan wrapper server.
reconnaissance/pd-tools-mcp/manifest.yaml	Adds wrapper manifest metadata for pd-tools wrapper server.
reconnaissance/networksdb-mcp/manifest.yaml	Adds wrapper manifest metadata for networksdb wrapper server.
reconnaissance/externalattacker-mcp/manifest.yaml	Adds wrapper manifest metadata for externalattacker wrapper server.
password-cracking/hashcat-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
osint/maigret-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
osint/dnstwist-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
meta/mcp-scan/manifest.yaml	Adds wrapper manifest metadata for mcp-scan wrapper server.
code-security/semgrep-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
cloud-security/roadrecon-mcp/manifest.yaml	Adds wrapper manifest metadata for roadrecon wrapper server.
binary-analysis/radare2-mcp/manifest.yaml	Adds wrapper manifest metadata for radare2 wrapper server.
binary-analysis/ida-mcp/manifest.yaml	Adds wrapper manifest metadata for ida wrapper server.
binary-analysis/ghidra-mcp/manifest.yaml	Adds wrapper manifest metadata for ghidra wrapper server.
active-directory/bloodhound-mcp/manifest.yaml	Adds wrapper manifest metadata (includes empty tools list).
tests/test_hub_servers.py	New tiered test suite using server auto-discovery and docker-marked smoke tests.
tests/mcp_harness.py	New Docker MCP JSON-RPC smoke-test harness for `initialize`/`tools/list`.
tests/discovery.py	New auto-discovery utility for locating full vs wrapper servers and reading manifests.
tests/conftest.py	Registers `docker` marker and provides docker availability fixture.
scripts/generate_manifests.py	Adds a scaffold generator for wrapper manifests.
pytest.ini	Registers `docker` marker in pytest configuration.
.github/workflows/validate.yml	Adds a dedicated validation workflow with optional/PR-targeted Docker smoke runs.
.github/workflows/build.yml	Updates CI test job to run new validation suite (tiers 1–3).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T01:27:39Z

+    if hasattr(module.app, "_tool_handlers"):
+        handler = module.app._tool_handlers.get("list_tools")
+        if handler:
+            return await handler()
+
+    pytest.skip("Cannot find list_tools method")


get_tools_from_module() accesses module.app without first checking that the module has an app attribute. Because pytest does not guarantee test execution order, a missing app could raise AttributeError here before test_server_has_app runs, causing an error instead of a clean assertion/skip. Consider guarding with hasattr(module, "app") (and failing with a clear message) before touching module.app.

Suggested change

if hasattr(module.app, "_tool_handlers"):

handler = module.app._tool_handlers.get("list_tools")

if handler:

return await handler()

pytest.skip("Cannot find list_tools method")

if hasattr(module, "app"):

if hasattr(module.app, "_tool_handlers"):

handler = module.app._tool_handlers.get("list_tools")

if handler:

return await handler()

pytest.fail("Module has an app attribute but does not expose a list_tools handler")

pytest.skip("Cannot find list_tools method or app attribute")

Copilot · 2026-04-08T01:27:40Z

+import importlib.util
+import re
+import sys
+from pathlib import Path
+from typing import Any
+
+import pytest
+
+# Ensure tests/ directory is on path for discovery/harness imports
+_TESTS_DIR = Path(__file__).resolve().parent
+if str(_TESTS_DIR) not in sys.path:
+    sys.path.insert(0, str(_TESTS_DIR))
+
+from discovery import MCPServer, discover_servers, full_servers
+


There are unused imports here (re and full_servers) which add noise and can hide real dependency issues. Please remove unused imports or use them (e.g., if you intended to validate name patterns via regex).

Copilot · 2026-04-08T01:27:40Z

+    module = load_server_module(server)
+    loop = asyncio.new_event_loop()
+    try:
+        tools = loop.run_until_complete(get_tools_from_module(module))
+    finally:


_tools_from_module_sync() creates a new event loop but never sets it as the current loop. If a server’s list_tools() implementation calls asyncio.get_event_loop() / get_running_loop() indirectly (common in older asyncio code), this can fail or behave differently than in normal execution. Consider calling asyncio.set_event_loop(loop) for the duration (and restoring the previous loop) to make extraction more reliable.

Suggested change

module = load_server_module(server)

loop = asyncio.new_event_loop()

try:

tools = loop.run_until_complete(get_tools_from_module(module))

finally:

module = load_server_module(server)

try:

previous_loop = asyncio.get_event_loop()

except RuntimeError:

previous_loop = None

loop = asyncio.new_event_loop()

try:

asyncio.set_event_loop(loop)

tools = loop.run_until_complete(get_tools_from_module(module))

finally:

asyncio.set_event_loop(previous_loop)

Copilot · 2026-04-08T01:27:40Z

+def _read_response(proc: subprocess.Popen, timeout: float = 30.0) -> dict[str, Any]:
+    """Read a JSON-RPC response from stdout, skipping notifications."""
+    deadline = time.monotonic() + timeout
+    while time.monotonic() < deadline:
+        line = proc.stdout.readline()
+        if not line:
+            # Check if process has died
+            if proc.poll() is not None:
+                stderr = proc.stderr.read() if proc.stderr else ""
+                raise MCPSmokeTestError(
+                    f"MCP server exited with code {proc.returncode}: {stderr[-2000:]}"
+                )
+            time.sleep(0.1)
+            continue


_read_response() uses blocking proc.stdout.readline(), which can block indefinitely and bypass the intended deadline timeout (the while condition won’t be re-checked until readline() returns). This can hang CI if a container never flushes a newline. Consider using non-blocking I/O (e.g., selectors/select on the stdout FD) or a background reader thread + queue so the timeout is actually enforced.

Copilot · 2026-04-08T01:27:40Z

+        if expected_tools:
+            for expected_tool in expected_tools:
+                name = expected_tool["name"]
+                if name not in tool_names:
+                    errors.append(f"Tool '{name}' expected but not returned by server")
+
+                # Check required params if tool exists
+                matching = [t for t in tools if t["name"] == name]
+                if matching:
+                    schema = matching[0].get("inputSchema", {})
+                    schema_required = set(schema.get("required", []))
+                    expected_required = set(expected_tool.get("required_params", []))
+                    missing_params = expected_required - schema_required
+                    if missing_params:
+                        errors.append(
+                            f"Tool '{name}' missing required params: {missing_params}"
+                        )
+
+        return {
+            "tools_found": sorted(tool_names),
+            "tools_expected": [t["name"] for t in expected_tools] if expected_tools else [],


run_mcp_smoke_test() treats expected_tools as a truthy check (if expected_tools:). Passing an empty list (e.g., because tool extraction failed) will skip all validation and still report success, which can hide regressions. Use an explicit expected_tools is not None check, and consider failing if a full server yields expected_tools == [] since Tier 2 expects tools to exist.

Suggested change

if expected_tools:

for expected_tool in expected_tools:

name = expected_tool["name"]

if name not in tool_names:

errors.append(f"Tool '{name}' expected but not returned by server")

# Check required params if tool exists

matching = [t for t in tools if t["name"] == name]

if matching:

schema = matching[0].get("inputSchema", {})

schema_required = set(schema.get("required", []))

expected_required = set(expected_tool.get("required_params", []))

missing_params = expected_required - schema_required

if missing_params:

errors.append(

f"Tool '{name}' missing required params: {missing_params}"

)

return {

"tools_found": sorted(tool_names),

"tools_expected": [t["name"] for t in expected_tools] if expected_tools else [],

if expected_tools is not None:

if not expected_tools:

errors.append(

"Expected tool definitions were provided as an empty list; "

"this skips validation and likely indicates tool extraction failed."

)

else:

for expected_tool in expected_tools:

name = expected_tool["name"]

if name not in tool_names:

errors.append(

f"Tool '{name}' expected but not returned by server"

)

# Check required params if tool exists

matching = [t for t in tools if t["name"] == name]

if matching:

schema = matching[0].get("inputSchema", {})

schema_required = set(schema.get("required", []))

expected_required = set(

expected_tool.get("required_params", [])

)

missing_params = expected_required - schema_required

if missing_params:

errors.append(

f"Tool '{name}' missing required params: {missing_params}"

)

return {

"tools_found": sorted(tool_names),

"tools_expected": [t["name"] for t in expected_tools]

if expected_tools is not None

else [],

Copilot · 2026-04-08T01:27:41Z

+        for category, name, _ in servers:
+            has_manifest = (_.parent / name / "manifest.yaml").exists() if False else (_ / "manifest.yaml").exists()


In --list mode, has_manifest = (_.parent / name / "manifest.yaml").exists() if False else (_ / "manifest.yaml").exists() contains dead/placeholder logic (if False) and an unnecessary alternate path. This is confusing and should be simplified to just check server_dir / "manifest.yaml".

Suggested change

for category, name, _ in servers:

has_manifest = (_.parent / name / "manifest.yaml").exists() if False else (_ / "manifest.yaml").exists()

for category, name, server_dir in servers:

has_manifest = (server_dir / "manifest.yaml").exists()

Copilot · 2026-04-08T01:27:41Z

+          # Get list of changed files in the PR
+          CHANGED=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)
+
+          # Extract unique server directories (category/name) that changed
+          SERVERS=$(echo "$CHANGED" | grep -oP '^[^/]+/[^/]+-mcp' | sort -u || true)
+
+          if [ -z "$SERVERS" ]; then
+            echo "has_changes=false" >> "$GITHUB_OUTPUT"
+            echo "servers=[]" >> "$GITHUB_OUTPUT"
+            echo "No MCP server changes detected"
+          else
+            # Build JSON array of {category, name, path} objects
+            JSON="["
+            FIRST=true
+            for SERVER in $SERVERS; do
+              CATEGORY=$(echo "$SERVER" | cut -d/ -f1)
+              NAME=$(echo "$SERVER" | cut -d/ -f2)
+              # Only include if Dockerfile exists
+              if [ -f "$SERVER/Dockerfile" ]; then
+                if [ "$FIRST" = true ]; then


grep -oP '^[^/]+/[^/]+-mcp' will miss valid server directories that don’t end with -mcp (e.g. meta/mcp-scan exists in this repo), so PR smoke tests won’t run when those servers change. Consider extracting the first two path components (cut -d/ -f1,2) and then filtering by presence of a Dockerfile instead of enforcing a -mcp suffix.

Copilot · 2026-04-08T01:27:41Z

+          pytest tests/test_hub_servers.py -m docker -v --tb=long \
+            -k "${{ matrix.server.name }}"


pytest -k "${{ matrix.server.name }}" is unsafe with names containing hyphens (e.g. nmap-mcp), because -k uses a boolean expression parser and nmap-mcp can be interpreted as nmap - mcp, leading to incorrect selection or filtering out the intended parametrized case. Consider generating a safe expression from the name (e.g., replace - with and) or avoid -k by passing the target server via an env var and filtering inside the test collection.

Suggested change

pytest tests/test_hub_servers.py -m docker -v --tb=long \

-k "${{ matrix.server.name }}"

server_name="${{ matrix.server.name }}"

safe_k_expr="${server_name//-/ and }"

pytest tests/test_hub_servers.py -m docker -v --tb=long \

-k "$safe_k_expr"

Copilot · 2026-04-08T01:27:41Z

+      - name: Run validation tests (Tiers 1-3)
        run: |
-          pytest tests/ -v --tb=short || true
+          pytest tests/test_hub_servers.py -m "not docker" -v --tb=short


This workflow now runs only tests/test_hub_servers.py, leaving tests/test_mcp_servers.py in the repo but no longer executed in CI. That can cause the old suite to silently rot and confuse contributors running pytest tests/ locally. Consider deleting/archiving the old test file or switching back to running the whole tests/ directory if the new suite is meant to fully replace it.

Suggested change

pytest tests/test_hub_servers.py -m "not docker" -v --tb=short

pytest tests/ -m "not docker" -v --tb=short

AFredefon added 2 commits March 30, 2026 01:23

feat(tests): add auto-discovery test system with MCP smoke tests

5ad1ebc

refactor(tests): remove manifest.yaml for full servers, keep only for…

a2b03b2

… wrappers

Copilot AI review requested due to automatic review settings April 8, 2026 01:21

Copilot started reviewing on behalf of AFredefon April 8, 2026 01:23 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feat/test system#27

Feat/test system#27
AFredefon wants to merge 2 commits into
masterfrom
feat/test-system

AFredefon commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		for category, name, _ in servers:
		has_manifest = (_.parent / name / "manifest.yaml").exists() if False else (_ / "manifest.yaml").exists()

		pytest tests/test_hub_servers.py -m docker -v --tb=long \
		-k "${{ matrix.server.name }}"

	pytest tests/test_hub_servers.py -m "not docker" -v --tb=short
	pytest tests/ -m "not docker" -v --tb=short

Uh oh!

Conversation

AFredefon commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants