-
-
Notifications
You must be signed in to change notification settings - Fork 2
Redesign fuzz engine #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Redesign fuzz engine #142
Conversation
WalkthroughRefactors the fuzz engine architecture by replacing monolithic Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/User
participant ToolExec as ToolExecutor
participant ToolMut as ToolMutator
participant AsyncExec as AsyncFuzzExecutor
participant ResultBld as ResultBuilder
participant Collector as ResultCollector
Client->>ToolExec: execute(tool, runs=N, phase)
activate ToolExec
loop For each run
ToolExec->>ToolMut: mutate(tool, phase)
activate ToolMut
ToolMut-->>ToolExec: mutated_args
deactivate ToolMut
ToolExec->>AsyncExec: execute(operation)
activate AsyncExec
AsyncExec-->>ToolExec: result/exception
deactivate AsyncExec
ToolExec->>ResultBld: build_tool_result(...)
activate ResultBld
ResultBld-->>ToolExec: result_dict
deactivate ResultBld
end
ToolExec->>Collector: collect_results({results, errors})
activate Collector
Collector-->>ToolExec: aggregated_results[]
deactivate Collector
ToolExec-->>Client: results[]
deactivate ToolExec
sequenceDiagram
participant Client as Client/User
participant ProtocolExec as ProtocolExecutor
participant ProtocolMut as ProtocolMutator
participant Transport as Transport/Network
participant AsyncExec as AsyncFuzzExecutor
participant ResultBld as ResultBuilder
participant Collector as ResultCollector
Client->>ProtocolExec: execute(protocol_type, runs=N, phase)
activate ProtocolExec
loop For each run
ProtocolExec->>ProtocolMut: mutate(protocol_type, phase)
activate ProtocolMut
ProtocolMut-->>ProtocolExec: fuzz_data
deactivate ProtocolMut
ProtocolExec->>Transport: send_raw(fuzz_data)
activate Transport
alt Success
Transport-->>ProtocolExec: server_response
else Error
Transport-->>ProtocolExec: exception
end
deactivate Transport
ProtocolExec->>ResultBld: build_protocol_result(...)
activate ResultBld
ResultBld-->>ProtocolExec: FuzzDataResult
deactivate ResultBld
end
ProtocolExec->>Collector: collect_results({results, errors})
activate Collector
Collector-->>ProtocolExec: aggregated_results[]
deactivate Collector
ProtocolExec-->>Client: results[]
deactivate ProtocolExec
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Documentation PreviewYour documentation changes have been built successfully! Preview URL: https://Agent-Hellboy.github.io/mcp-server-fuzzer/ The documentation will be automatically deployed to the main site when this PR is merged. This comment was automatically generated by the documentation workflow. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
==========================================
+ Coverage 69.94% 72.76% +2.81%
==========================================
Files 108 118 +10
Lines 6905 7028 +123
Branches 1043 1056 +13
==========================================
+ Hits 4830 5114 +284
+ Misses 1764 1603 -161
Partials 311 311 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Documentation PreviewYour documentation changes have been built successfully! Preview URL: https://Agent-Hellboy.github.io/mcp-server-fuzzer/ The documentation will be automatically deployed to the main site when this PR is merged. This comment was automatically generated by the documentation workflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/development/reference.md (1)
838-874: Update package layout example to reflect Mutators/Executors instead of legacy fuzzersThe “Package Layout and Fuzz Engine” tree still shows
fuzz_engine/fuzzer/*.py,fuzz_engine/strategy/…, andfuzz_engine/invariants.py, plus bullets that talk about “Fuzzer” as the orchestration layer. With the refactor, these responsibilities have moved undermutators/andexecutor/(with invariants underexecutor.invariants).To avoid confusing readers, consider updating that block to something like:
- fuzz_engine/ - fuzzer/ - protocol_fuzzer.py # Orchestrates protocol-type fuzzing - tool_fuzzer.py # Orchestrates tool fuzzing - strategy/ - schema_parser.py # JSON Schema parser for test data generation - strategy_manager.py # Selects strategies per phase/type + fuzz_engine/ + mutators/ + tool_mutator.py # Generates fuzzed tool arguments + protocol_mutator.py # Generates fuzzed protocol envelopes + batch_mutator.py # Generates JSON-RPC batch requests + strategies/ + schema_parser.py # JSON Schema parser for test data generation + strategy_manager.py # Realistic/aggressive strategy selection - invariants.py # Property-based invariants and checks + executor/ + tool_executor.py # Orchestrates tool fuzzing (uses AsyncFuzzExecutor) + protocol_executor.py # Orchestrates protocol-type fuzzing + invariants + batch_executor.py # Orchestrates batch fuzzing + invariants.py # Property-based invariants and checksand adjust the nearby bullet (
Strategy/Fuzzer/Runtime) to use “Mutators” / “Executors” terminology for consistency with the rest of the doc.Also applies to: 963-974
mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (2)
184-196: Inconsistentrunvalue in error results.When processing errors from batch execution,
runis set to0(line 191), but in_execute_single_runerror handling (line 288),runis set torun_index + 1. This inconsistency may confuse consumers of the results. Since the error occurred at the batch level without a specific run index, consider using a sentinel value or documenting this behavior.results.append( { "protocol_type": protocol_type, - "run": 0, + "run": -1, # Sentinel value indicating batch-level error "fuzz_data": {}, "success": False, "exception": str(error), } )Alternatively, you could track the operation index and map it to the run number.
397-416: Sequential awaiting of pre-created tasks may cause suboptimal timeout behavior.Tasks are created upfront (line 398) but awaited sequentially (lines 402-414). If an early task times out after 30 seconds, later tasks that may have already completed must still wait for their turn to be awaited. Consider using
asyncio.as_completed()for more responsive timeout handling, though the current approach is functionally correct.- for protocol_type, task in tasks: - try: - # Add a timeout to prevent hanging indefinitely - results = await asyncio.wait_for(task, timeout=30.0) - all_results[protocol_type] = results - except asyncio.TimeoutError: - self._logger.error(f"Timeout while fuzzing {protocol_type}") - all_results[protocol_type] = [] - # Cancel the task to avoid orphaned tasks - task.cancel() - except Exception as e: - self._logger.error(f"Failed to fuzz {protocol_type}: {e}") - all_results[protocol_type] = [] + # Use as_completed for more responsive processing + task_map = {task: pt for pt, task in tasks} + for coro in asyncio.as_completed([t for _, t in tasks], timeout=30.0 * len(tasks)): + try: + task = await coro + # Note: as_completed doesn't directly give the task, would need wrapper + except Exception as e: + self._logger.error(f"Task failed: {e}")Note: The above is a sketch;
as_completedrequires additional mapping logic to associate results with protocol types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (53)
.gitignore(1 hunks)docs/README.md(2 hunks)docs/architecture/architecture.md(4 hunks)docs/architecture/async-executor.md(1 hunks)docs/architecture/fuzz-engine.md(1 hunks)docs/development/reference.md(5 hunks)docs/index.md(1 hunks)mcp_fuzzer/__init__.py(1 hunks)mcp_fuzzer/client/protocol_client.py(5 hunks)mcp_fuzzer/client/tool_client.py(5 hunks)mcp_fuzzer/fuzz_engine/__init__.py(1 hunks)mcp_fuzzer/fuzz_engine/executor/__init__.py(1 hunks)mcp_fuzzer/fuzz_engine/executor/batch_executor.py(1 hunks)mcp_fuzzer/fuzz_engine/executor/protocol_executor.py(17 hunks)mcp_fuzzer/fuzz_engine/executor/tool_executor.py(10 hunks)mcp_fuzzer/fuzz_engine/fuzzer/__init__.py(0 hunks)mcp_fuzzer/fuzz_engine/fuzzerreporter/__init__.py(1 hunks)mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py(1 hunks)mcp_fuzzer/fuzz_engine/fuzzerreporter/metrics.py(1 hunks)mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/__init__.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/base.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/strategies/realistic/protocol_type_strategy.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py(1 hunks)mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py(1 hunks)tests/add_markers.py(1 hunks)tests/unit/fuzz_engine/executor/__init__.py(1 hunks)tests/unit/fuzz_engine/executor/test_async_executor.py(1 hunks)tests/unit/fuzz_engine/executor/test_batch_executor.py(1 hunks)tests/unit/fuzz_engine/executor/test_invariants.py(8 hunks)tests/unit/fuzz_engine/executor/test_protocol_executor.py(1 hunks)tests/unit/fuzz_engine/executor/test_tool_executor.py(1 hunks)tests/unit/fuzz_engine/fuzzer/test_protocol_fuzzer.py(6 hunks)tests/unit/fuzz_engine/fuzzer/test_tool_fuzzer.py(7 hunks)tests/unit/fuzz_engine/fuzzerreporter/__init__.py(1 hunks)tests/unit/fuzz_engine/fuzzerreporter/test_collector.py(1 hunks)tests/unit/fuzz_engine/fuzzerreporter/test_metrics.py(1 hunks)tests/unit/fuzz_engine/fuzzerreporter/test_result_builder.py(1 hunks)tests/unit/fuzz_engine/mutators/__init__.py(1 hunks)tests/unit/fuzz_engine/mutators/test_base.py(1 hunks)tests/unit/fuzz_engine/mutators/test_batch_mutator.py(1 hunks)tests/unit/fuzz_engine/mutators/test_protocol_mutator.py(1 hunks)tests/unit/fuzz_engine/mutators/test_schema_parser.py(1 hunks)tests/unit/fuzz_engine/mutators/test_schema_parser_advanced.py(1 hunks)tests/unit/fuzz_engine/mutators/test_tool_mutator.py(1 hunks)tests/unit/fuzz_engine/strategy/test_aggressive_protocol_strategies.py(4 hunks)tests/unit/fuzz_engine/strategy/test_realistic_strategies.py(6 hunks)tests/unit/fuzz_engine/strategy/test_schema_parser.py(1 hunks)tests/unit/fuzz_engine/strategy/test_strategy_manager_protocol.py(1 hunks)tests/unit/fuzz_engine/strategy/test_strategy_manager_tool.py(1 hunks)tests/unit/fuzz_engine/test_invariants.py(8 hunks)
💤 Files with no reviewable changes (1)
- mcp_fuzzer/fuzz_engine/fuzzer/init.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Agent-Hellboy
Repo: Agent-Hellboy/mcp-server-fuzzer PR: 25
File: mcp_fuzzer/strategy/tool_strategies.py:105-139
Timestamp: 2025-08-08T11:50:01.877Z
Learning: Repo Agent-Hellboy/mcp-server-fuzzer: In mcp_fuzzer/strategy/tool_strategies.py, ToolStrategies._generate_aggressive_integer can return None in the "special" branch despite an int annotation. Maintainership decision: ignore for now and handle in issue #18; do not re-flag this as blocking in interim reviews.
📚 Learning: 2025-08-08T11:50:01.877Z
Learnt from: Agent-Hellboy
Repo: Agent-Hellboy/mcp-server-fuzzer PR: 25
File: mcp_fuzzer/strategy/tool_strategies.py:105-139
Timestamp: 2025-08-08T11:50:01.877Z
Learning: Repo Agent-Hellboy/mcp-server-fuzzer: In mcp_fuzzer/strategy/tool_strategies.py, ToolStrategies._generate_aggressive_integer can return None in the "special" branch despite an int annotation. Maintainership decision: ignore for now and handle in issue #18; do not re-flag this as blocking in interim reviews.
Applied to files:
tests/unit/fuzz_engine/strategy/test_strategy_manager_tool.pymcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.pytests/unit/fuzz_engine/strategy/test_realistic_strategies.pymcp_fuzzer/fuzz_engine/__init__.pytests/unit/fuzz_engine/strategy/test_aggressive_protocol_strategies.pymcp_fuzzer/client/tool_client.pytests/unit/fuzz_engine/fuzzer/test_tool_fuzzer.py
🧬 Code graph analysis (30)
mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (2)
mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (2)
ProtocolStrategies(31-240)generate_batch_request(105-196)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
mutate(37-70)
tests/unit/fuzz_engine/mutators/test_base.py (1)
mcp_fuzzer/fuzz_engine/mutators/base.py (2)
Mutator(12-27)mutate(16-27)
tests/unit/fuzz_engine/test_invariants.py (2)
tests/unit/fuzz_engine/executor/test_async_executor.py (1)
executor(16-18)tests/unit/fuzz_engine/test_executor.py (1)
executor(14-16)
mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (2)
mcp_fuzzer/fuzz_engine/mutators/base.py (2)
Mutator(12-27)mutate(16-27)mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (2)
ToolStrategies(242-256)fuzz_tool_arguments(249-256)
tests/unit/fuzz_engine/mutators/test_batch_mutator.py (5)
mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (2)
BatchMutator(14-38)mutate(21-38)mcp_fuzzer/fuzz_engine/mutators/base.py (1)
mutate(16-27)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
mutate(37-70)tests/unit/fuzz_engine/mutators/test_protocol_mutator.py (1)
test_mutate_realistic_phase(52-56)tests/unit/fuzz_engine/mutators/test_tool_mutator.py (2)
test_mutate_realistic_phase(43-47)test_mutate_strategies_integration(94-102)
tests/unit/fuzz_engine/executor/test_protocol_executor.py (5)
mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (5)
ProtocolExecutor(23-524)execute(118-157)execute_both_phases(340-369)execute_all_types(371-416)shutdown(522-524)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
ProtocolMutator(15-70)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
BatchMutator(14-38)mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (1)
ResultBuilder(13-139)mcp_fuzzer/transport/base.py (2)
send_batch_request(88-123)collate_batch_responses(125-162)
tests/unit/fuzz_engine/strategy/test_strategy_manager_protocol.py (1)
mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (1)
ProtocolStrategies(31-240)
tests/unit/fuzz_engine/fuzzerreporter/test_metrics.py (1)
mcp_fuzzer/fuzz_engine/fuzzerreporter/metrics.py (3)
MetricsCalculator(11-61)calculate_tool_metrics(14-35)calculate_protocol_metrics(37-61)
tests/unit/fuzz_engine/executor/test_async_executor.py (1)
mcp_fuzzer/fuzz_engine/executor/async_executor.py (6)
AsyncFuzzExecutor(17-121)_get_semaphore(32-36)execute_batch(38-77)run_hypothesis_strategy(106-117)shutdown(119-121)_execute_single(79-104)
mcp_fuzzer/__init__.py (6)
mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (1)
ToolMutator(14-34)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
ProtocolMutator(15-70)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
BatchMutator(14-38)mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (1)
ProtocolExecutor(23-524)mcp_fuzzer/fuzz_engine/executor/batch_executor.py (1)
BatchExecutor(17-150)mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (2)
ProtocolStrategies(31-240)ToolStrategies(242-256)
mcp_fuzzer/fuzz_engine/executor/batch_executor.py (6)
mcp_fuzzer/types.py (1)
FuzzDataResult(14-26)mcp_fuzzer/fuzz_engine/executor/async_executor.py (2)
AsyncFuzzExecutor(17-121)shutdown(119-121)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (2)
BatchMutator(14-38)mutate(21-38)mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (2)
ResultBuilder(13-139)build_batch_result(106-139)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
mutate(37-70)mcp_fuzzer/transport/base.py (2)
send_batch_request(88-123)collate_batch_responses(125-162)
tests/unit/fuzz_engine/fuzzerreporter/test_collector.py (1)
mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py (2)
collect_results(14-38)filter_results(40-55)
tests/unit/fuzz_engine/fuzzer/test_protocol_fuzzer.py (2)
mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (3)
ProtocolExecutor(23-524)execute(118-157)execute_all_types(371-416)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
ProtocolMutator(15-70)
mcp_fuzzer/fuzz_engine/mutators/__init__.py (5)
mcp_fuzzer/fuzz_engine/mutators/base.py (1)
Mutator(12-27)mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (1)
ToolMutator(14-34)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
ProtocolMutator(15-70)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
BatchMutator(14-38)mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (2)
ProtocolStrategies(31-240)ToolStrategies(242-256)
tests/unit/fuzz_engine/mutators/test_protocol_mutator.py (2)
mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (3)
ProtocolMutator(15-70)get_fuzzer_method(22-35)mutate(37-70)mcp_fuzzer/fuzz_engine/mutators/base.py (1)
mutate(16-27)
tests/unit/fuzz_engine/mutators/test_schema_parser_advanced.py (1)
mcp_fuzzer/fuzz_engine/mutators/strategies/schema_parser.py (7)
make_fuzz_strategy_from_jsonschema(130-227)_merge_allOf(31-128)_handle_string_type(396-449)_handle_integer_type(584-649)_handle_number_type(651-725)_handle_array_type(317-394)_handle_object_type(262-315)
tests/unit/fuzz_engine/strategy/test_realistic_strategies.py (1)
mcp_fuzzer/fuzz_engine/mutators/strategies/realistic/tool_strategy.py (5)
base64_strings(19-42)timestamp_strings(76-100)uuid_strings(44-74)generate_realistic_text(102-158)fuzz_tool_arguments_realistic(160-310)
mcp_fuzzer/fuzz_engine/mutators/base.py (3)
mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
mutate(21-38)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
mutate(37-70)mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (1)
mutate(21-34)
mcp_fuzzer/fuzz_engine/fuzzerreporter/__init__.py (5)
tests/unit/fuzz_engine/fuzzerreporter/test_result_builder.py (1)
result_builder(18-20)mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (1)
ResultBuilder(13-139)tests/unit/fuzz_engine/fuzzerreporter/test_collector.py (1)
collector(18-20)mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py (1)
ResultCollector(11-55)mcp_fuzzer/fuzz_engine/fuzzerreporter/metrics.py (1)
MetricsCalculator(11-61)
tests/unit/fuzz_engine/fuzzerreporter/test_result_builder.py (2)
mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (4)
ResultBuilder(13-139)build_tool_result(16-68)build_protocol_result(70-104)build_batch_result(106-139)mcp_fuzzer/reports/core/models.py (1)
safety_blocked(29-30)
mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (2)
mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (2)
ProtocolStrategies(31-240)get_protocol_fuzzer_method(82-102)mcp_fuzzer/fuzz_engine/mutators/strategies/aggressive/protocol_type_strategy.py (1)
get_protocol_fuzzer_method(871-926)
mcp_fuzzer/client/protocol_client.py (3)
mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (2)
ProtocolMutator(15-70)mutate(37-70)mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (2)
ProtocolExecutor(23-524)shutdown(522-524)mcp_fuzzer/fuzz_engine/mutators/base.py (1)
mutate(16-27)
mcp_fuzzer/fuzz_engine/__init__.py (11)
mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (1)
ToolMutator(14-34)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
ProtocolMutator(15-70)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
BatchMutator(14-38)mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (2)
ProtocolStrategies(31-240)ToolStrategies(242-256)mcp_fuzzer/fuzz_engine/executor/async_executor.py (1)
AsyncFuzzExecutor(17-121)mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (1)
ProtocolExecutor(23-524)mcp_fuzzer/fuzz_engine/executor/batch_executor.py (1)
BatchExecutor(17-150)mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (1)
ResultBuilder(13-139)mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py (1)
ResultCollector(11-55)mcp_fuzzer/fuzz_engine/fuzzerreporter/metrics.py (1)
MetricsCalculator(11-61)mcp_fuzzer/fuzz_engine/runtime/watchdog.py (1)
ProcessWatchdog(305-508)
tests/unit/fuzz_engine/mutators/test_tool_mutator.py (4)
mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (2)
ToolMutator(14-34)mutate(21-34)mcp_fuzzer/fuzz_engine/mutators/base.py (1)
mutate(16-27)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
mutate(21-38)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
mutate(37-70)
mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (2)
mcp_fuzzer/types.py (1)
FuzzDataResult(14-26)mcp_fuzzer/reports/core/models.py (1)
safety_blocked(29-30)
mcp_fuzzer/fuzz_engine/executor/__init__.py (5)
mcp_fuzzer/fuzz_engine/executor/async_executor.py (1)
AsyncFuzzExecutor(17-121)mcp_fuzzer/fuzz_engine/executor/tool_executor.py (1)
ToolExecutor(18-274)mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (1)
ProtocolExecutor(23-524)mcp_fuzzer/fuzz_engine/executor/batch_executor.py (1)
BatchExecutor(17-150)mcp_fuzzer/fuzz_engine/executor/invariants.py (7)
InvariantViolation(39-45)check_response_validity(47-130)check_error_type_correctness(132-188)check_response_schema_conformity(190-214)verify_response_invariants(216-246)verify_batch_responses(248-302)check_state_consistency(304-344)
tests/unit/fuzz_engine/strategy/test_aggressive_protocol_strategies.py (3)
mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (1)
ProtocolExecutor(23-524)mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (1)
get_protocol_fuzzer_method(82-102)mcp_fuzzer/fuzz_engine/mutators/strategies/aggressive/protocol_type_strategy.py (1)
get_protocol_fuzzer_method(871-926)
mcp_fuzzer/client/tool_client.py (5)
mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (2)
ToolMutator(14-34)mutate(21-34)tests/unit/fuzz_engine/mutators/test_tool_mutator.py (1)
tool_mutator(15-17)mcp_fuzzer/fuzz_engine/mutators/base.py (1)
mutate(16-27)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (1)
mutate(21-38)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
mutate(37-70)
tests/unit/fuzz_engine/fuzzer/test_tool_fuzzer.py (3)
mcp_fuzzer/fuzz_engine/executor/tool_executor.py (4)
ToolExecutor(18-274)execute(51-93)execute_multiple(195-234)execute_both_phases(163-193)mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py (2)
ToolMutator(14-34)mutate(21-34)mcp_fuzzer/fuzz_engine/mutators/base.py (1)
mutate(16-27)
mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (6)
mcp_fuzzer/types.py (1)
FuzzDataResult(14-26)mcp_fuzzer/fuzz_engine/executor/async_executor.py (2)
AsyncFuzzExecutor(17-121)shutdown(119-121)mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (3)
ProtocolMutator(15-70)get_fuzzer_method(22-35)mutate(37-70)mcp_fuzzer/fuzz_engine/mutators/batch_mutator.py (2)
BatchMutator(14-38)mutate(21-38)mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (3)
ResultBuilder(13-139)build_protocol_result(70-104)build_batch_result(106-139)mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py (1)
ResultCollector(11-55)
🪛 LanguageTool
docs/index.md
[style] ~109-~109: This phrase is redundant (‘I’ stands for ‘Interface’). Use simply “CLIInterface”.
Context: ... process management and monitoring - CLI Interface - User-friendly command-line interfac...
(ACRONYM_TAUTOLOGY)
🪛 markdownlint-cli2 (0.18.1)
docs/index.md
115-115: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
119-119: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
125-125: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
129-129: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
134-134: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
139-139: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (57)
.gitignore (1)
21-21: LGTM!The addition of
*USAGE.mdto the ignore patterns is consistent with the existing markdown documentation patterns (e.g.,*REDESIGN*.md,*FLOW.md) and aligns with the PR objective to manage documentation during the engine redesign.tests/add_markers.py (1)
18-21: All three new directory paths exist and match the COMPONENT_MARKERS entries exactly.Verification confirms that:
tests/unit/fuzz_engine/executor/exists ✓tests/unit/fuzz_engine/fuzzerreporter/exists (correctly named without underscore) ✓tests/unit/fuzz_engine/mutators/exists ✓The new marker mappings are properly configured and will function correctly with the add_markers.py script logic.
tests/unit/fuzz_engine/strategy/test_schema_parser.py (1)
11-20: Import path update aligns with new mutators layoutSwitching to
mcp_fuzzer.fuzz_engine.mutators.strategies.schema_parserkeeps the same tested API while matching the redesigned fuzz engine structure. No behavioral change to the tests.tests/unit/fuzz_engine/mutators/test_schema_parser.py (1)
10-19: Updated schema_parser import is consistent with new module structureImporting from
mcp_fuzzer.fuzz_engine.mutators.strategies.schema_parserkeeps coverage on the same helpers while aligning with the new mutator-based layout.tests/unit/fuzz_engine/mutators/test_schema_parser_advanced.py (1)
1-391: Overall: strong, targeted coverage of schema_parser edge casesStepping back, this file substantially improves coverage for
_merge_allOf, numeric constraints, formats, const handling, recursion, and aggressive behavior. Aside from the few opportunities to tighten assertions and clarify docstrings, the structure and intent of these tests look solid.mcp_fuzzer/fuzz_engine/mutators/strategies/strategy_manager.py (1)
145-148: LGTM: Appropriate guard clause for empty protocol types.The early return correctly handles the edge case where an explicit empty list is passed, distinguishing it from
None(which uses defaults). This is clear, intentional behavior.mcp_fuzzer/fuzz_engine/mutators/base.py (1)
12-27: Well-designed abstract base class for the mutator hierarchy.The flexible
*args, **kwargssignature appropriately accommodates the different parameter requirements ofToolMutator,ProtocolMutator, andBatchMutatorwhile establishing a clear interface contract.tests/unit/fuzz_engine/strategy/test_realistic_strategies.py (1)
4-5: Import paths correctly updated to the new module structure.The imports now correctly reference the relocated strategy modules under
mcp_fuzzer.fuzz_engine.mutators.strategies.realistic.*.Also applies to: 16-27
mcp_fuzzer/fuzz_engine/mutators/protocol_mutator.py (1)
37-70: LGTM: Well-implemented async/sync compatibility.The
mutatemethod correctly handles both synchronous and asynchronous fuzzer methods usinginspect.isawaitable. The phase parameter introspection is a nice touch for backward compatibility with fuzzer methods that don't accept a phase argument.tests/unit/fuzz_engine/strategy/test_aggressive_protocol_strategies.py (2)
3-5: Import paths correctly updated to the new module structure.The imports now reference the relocated aggressive strategy modules and the renamed
ProtocolExecutorclass (replacingProtocolFuzzer).Also applies to: 10-19
265-342: Good regression test for PROTOCOL_TYPES synchronization.This test ensures
ProtocolExecutor.PROTOCOL_TYPESstays in sync with the fuzzer method map, which is valuable for maintaining consistency as the protocol type list evolves.mcp_fuzzer/fuzz_engine/mutators/strategies/realistic/protocol_type_strategy.py (1)
13-13: Import path is correct and properly resolves to the config package.The
.....configimport correctly points tomcp_fuzzer/config/, which is a package that exportsDEFAULT_PROTOCOL_VERSIONfrom its__init__.py. This import pattern matches existing usage in the codebase (e.g.,mcp_fuzzer/transport/streamable_http.py).mcp_fuzzer/fuzz_engine/fuzzerreporter/result_builder.py (3)
16-68: LGTM!The
build_tool_resultmethod correctly builds a standardized tool result dictionary with conditional field inclusion. The logic for handling optional fields and safety-related flags is clean and well-documented.
70-104: LGTM!The
build_protocol_resultmethod correctly constructs aFuzzDataResultwith proper defaults forinvariant_violations. The success/rejection logic tied toserver_erroris clear.
127-137: Perfect! Now let me examine theFuzzDataResultTypedDict definition:
<function_calls>
cat -n mcp_fuzzer/types.py
</function_calls>docs/README.md (1)
15-15: LGTM!Documentation structure correctly updated to include the new fuzz engine architecture document with a corresponding quick navigation entry.
Also applies to: 43-43
tests/unit/fuzz_engine/mutators/test_base.py (1)
13-22: LGTM!Tests correctly validate the abstract base class contract: ensuring
Mutatorcannot be instantiated and thatmutateis defined as an abstract method. Good use of__isabstractmethod__introspection.tests/unit/fuzz_engine/mutators/__init__.py (1)
1-5: LGTM!Standard package initializer for the mutators test module.
tests/unit/fuzz_engine/strategy/test_strategy_manager_tool.py (1)
10-10: LGTM!Import path correctly updated to reflect the module reorganization from
mcp_fuzzer.fuzz_engine.strategytomcp_fuzzer.fuzz_engine.mutators.strategies.tests/unit/fuzz_engine/strategy/test_strategy_manager_protocol.py (1)
9-9: LGTM! Import path updated to reflect new architecture.The import path change from
mcp_fuzzer.fuzz_engine.strategytomcp_fuzzer.fuzz_engine.mutators.strategiesaligns with the broader refactoring that relocates strategies under the mutators module.tests/unit/fuzz_engine/fuzzerreporter/__init__.py (1)
1-5: LGTM! Standard test package initializer.This provides the necessary package structure for the fuzzerreporter unit tests.
mcp_fuzzer/fuzz_engine/executor/__init__.py (1)
1-34: LGTM! Well-organized executor package surface.The package initializer properly exports all executor-related components (AsyncFuzzExecutor, ToolExecutor, ProtocolExecutor, BatchExecutor) and invariant validation utilities, providing a clean public API for the execution and orchestration layer.
tests/unit/fuzz_engine/mutators/test_protocol_mutator.py (1)
1-147: LGTM! Comprehensive test coverage for ProtocolMutator.The test suite thoroughly validates ProtocolMutator behavior across initialization, fuzzer method retrieval, mutation phases, async/sync execution paths, phase parameter handling, and error cases for unknown protocol types.
tests/unit/fuzz_engine/executor/test_async_executor.py (1)
1-217: LGTM! Thorough test coverage for AsyncFuzzExecutor.The test suite comprehensively validates AsyncFuzzExecutor behavior including initialization, lazy semaphore creation, batch execution with various scenarios (success, exceptions, cancellation), concurrency limits, Hypothesis integration, shutdown, and single operation execution for both async and sync paths.
tests/unit/fuzz_engine/mutators/test_tool_mutator.py (1)
1-103: LGTM! Solid test coverage for ToolMutator.The test suite validates ToolMutator behavior across initialization, phase-specific mutations (realistic/aggressive/default), edge cases (empty tool, complex schema), and proper delegation to underlying strategies.
tests/unit/fuzz_engine/mutators/test_batch_mutator.py (1)
1-104: LGTM! Comprehensive test coverage for BatchMutator.The test suite validates BatchMutator behavior across initialization, mutation with/without protocol types, phase-specific mutations, delegation to strategies, and edge cases including empty and multiple protocol types.
mcp_fuzzer/fuzz_engine/fuzzerreporter/__init__.py (1)
1-16: LGTM! Clean fuzzerreporter package surface.The package initializer properly exports the three core reporting components (ResultBuilder, ResultCollector, MetricsCalculator), providing a well-defined public API for result collection and reporting.
tests/unit/fuzz_engine/test_invariants.py (1)
10-18: Invariants import and patch paths look consistent with new executor layoutThe switch to importing invariants helpers from
mcp_fuzzer.fuzz_engine.executorand patchingmcp_fuzzer.fuzz_engine.executor.invariants.*is consistent across the tests and matches the new module organization. No issues from the test side assumingexecutor.__init__re‑exports these symbols and importsinvariantsas expected.Also applies to: 147-188, 200-270
tests/unit/fuzz_engine/fuzzerreporter/test_collector.py (1)
23-112: Good coverage of ResultCollector behaviorThe tests cover the key
ResultCollectorpaths (pure successes, mixed errors,Nonefiltering, empty/missing keys, andsuccess_onlyfiltering) and match the current implementation’s semantics.tests/unit/fuzz_engine/executor/test_invariants.py (1)
10-18: Executor invariants paths updated consistentlyThe imports and
patch()targets now all referencemcp_fuzzer.fuzz_engine.executor.invariants.*, matching the new layout and mirroring the pytest-based tests. This keeps the unittest suite aligned with the executor module without changing behavior.Also applies to: 133-188, 181-219, 228-248
docs/architecture/async-executor.md (1)
114-123: AsyncFuzzExecutor integration section matches the new architectureThe updated bullets clearly position
AsyncFuzzExecutorbetween mutators, the executor layer, and theFuzzerReportercomponents, and they stay consistent with the rest of the docs/PR terminology.tests/unit/fuzz_engine/fuzzerreporter/test_metrics.py (1)
23-127: MetricsCalculator tests accurately exercise success/rejection metricsThe scenarios here (success-only, failure-only, mixed, empty, and missing
server_rejected_input) line up well withMetricsCalculator’s implementation and should catch regressions in both rates and count fields.tests/unit/fuzz_engine/executor/test_protocol_executor.py (1)
37-212: Solid coverage of ProtocolExecutor behaviorThis suite exercises the key paths (success, invalid runs, unknown protocol, generate_only, transport failures, invariants, batch handling, cancellation, shutdown) in a way that closely matches
ProtocolExecutor’s logic. Assertions line up withResultBuilder’s output shape and the invariants flow, and the fixture setup is appropriate for async testing.tests/unit/fuzz_engine/fuzzerreporter/test_result_builder.py (1)
23-205: ResultBuilder behavior is well coveredThese tests comprehensively exercise
ResultBuilderfor tool, protocol, and batch results, including run indexing, optional fields, safety flags, server rejection flags, invariant-violation handling, and empty batches. They tightly match the current implementation and should guard against regressions in result shape.mcp_fuzzer/fuzz_engine/mutators/__init__.py (1)
1-20: Clean mutators aggregation moduleThis module neatly centralizes exports for mutators and strategies (
Mutator,ToolMutator,ProtocolMutator,BatchMutator,ProtocolStrategies,ToolStrategies), makingmcp_fuzzer.fuzz_engine.mutatorsa convenient import surface. No issues spotted.docs/development/reference.md (1)
892-910: Updated references to schema parser, invariants, and executors look correctThe new module paths for the schema parser (
mcp_fuzzer.fuzz_engine.mutators.strategies.schema_parser) and invariants (mcp_fuzzer.fuzz_engine.executor.invariants), along with the lifecycle description usingToolExecutor,ProtocolExecutor, andAsyncFuzzExecutor, all match the refactored code structure. The added link to the dedicated fuzz engine architecture doc is also helpful for discoverability.Also applies to: 930-948, 969-978
mcp_fuzzer/client/protocol_client.py (1)
15-16: ProtocolClient now correctly delegates protocol data generation to ProtocolMutatorSwitching
_process_single_protocol_fuzzto callself.protocol_mutator.mutate(protocol_type, phase="aggressive")keeps the client responsible for safety + transport while reusing the shared protocol strategies viaProtocolMutator. This is a clean separation of concerns and aligns with the new Mutators/Executors architecture.Also applies to: 39-43, 124-127
mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py (1)
11-56: LGTM!The
ResultCollectorimplementation is clean and straightforward. The use of.get()with default values provides safe dictionary access, and the filtering logic is clear and concise.tests/unit/fuzz_engine/fuzzer/test_protocol_fuzzer.py (1)
12-144: LGTM!The test updates correctly align with the new
ProtocolExecutorAPI. All method calls have been updated toexecuteandexecute_all_types, patch paths reference the correct module (protocol_executor), and test coverage is maintained.mcp_fuzzer/fuzz_engine/__init__.py (1)
11-52: LGTM!The public API reorganization is well-structured and comprehensive. The separation into Mutators, Executors, FuzzerReporter, and Runtime sections provides clear categorization, and all new components are properly exposed.
mcp_fuzzer/fuzz_engine/executor/batch_executor.py (3)
20-42: LGTM!The initialization is well-structured with proper dependency injection and sensible defaults for all components.
111-146: LGTM!The
_send_batch_requestmethod cleanly handles transport communication with proper error capture and logging.
148-150: LGTM!Clean shutdown delegation to the underlying executor.
tests/unit/fuzz_engine/fuzzer/test_tool_fuzzer.py (1)
9-124: LGTM!The test updates correctly migrate to the new
ToolExecutorandToolMutatorarchitecture. All tests properly use theexecutefamily of methods, maintain safety integration, and preserve comprehensive coverage.tests/unit/fuzz_engine/executor/test_batch_executor.py (1)
1-155: LGTM!Comprehensive test suite for
BatchExecutorwith excellent coverage of normal operation, edge cases, error handling, and shutdown behavior. The tests are well-structured with proper fixtures and assertions.mcp_fuzzer/fuzz_engine/executor/tool_executor.py (1)
18-274: LGTM!The
ToolExecutorrefactoring is well-executed with clean separation of concerns. The integration ofToolMutator,AsyncFuzzExecutor,ResultBuilder, andResultCollectorcomponents follows good dependency injection patterns. Safety integration is properly maintained, and error handling is comprehensive throughout.tests/unit/fuzz_engine/executor/test_tool_executor.py (1)
1-220: LGTM!Comprehensive test suite for
ToolExecutorwith excellent coverage of initialization, execution flows, safety integration, error handling, multi-tool scenarios, and phase management. The tests are well-organized with clear fixtures and thorough assertions.docs/architecture/architecture.md (4)
39-43: LGTM!The diagram components are updated consistently to reflect the new naming convention (ToolExecutor, ProtocolExecutor, Mutators, FuzzerReporter).
104-108: LGTM!Data flow diagram correctly reflects the new mutator-based architecture with ToolMutator and ProtocolMutator generating test data.
171-209: LGTM!The project structure documentation is well-organized and clearly reflects the three-tier modular design (mutators, executor, fuzzerreporter) with appropriate subdirectories for strategies and invariants.
299-332: Referenced documentation file exists and is correctly located.The
./fuzz-engine.mdfile is present atdocs/architecture/fuzz-engine.md, confirming the cross-reference in the fuzzing process documentation is valid. No issues identified.mcp_fuzzer/fuzz_engine/executor/protocol_executor.py (6)
118-157: LGTM!The
executemethod properly validates input, delegates to the mutator for fuzzer method resolution, and constructs operations for batch execution.
200-292: LGTM!The
_execute_single_runmethod has well-structured error handling:
- Proper timeout handling for batch invariant validation
- Correct re-raising of
CancelledError- Good use of
result_builderfor standardized result creation- The batch response detection heuristic (checking for list or dict without
jsonrpckey) is reasonable.
294-338: LGTM!The
_send_fuzzed_requestmethod cleanly separates transport concerns, properly handles both batch and single requests, and has appropriate exception handling.
340-369: LGTM!The
execute_both_phasesmethod correctly orchestrates sequential execution of realistic and aggressive fuzzing phases.
418-454: LGTM!The
_execute_single_typemethod provides a useful logging wrapper aroundexecutewith clear statistics reporting.
522-525: LGTM!The shutdown method correctly delegates cleanup to the underlying executor.
| #### ToolMutator | ||
|
|
||
| Generates fuzzed arguments for MCP tools based on their JSON Schema specifications. | ||
|
|
||
| ```python | ||
| class ToolMutator(BaseMutator): | ||
| """Generates fuzzed tool arguments.""" | ||
|
|
||
| def __init__(self): | ||
| self.strategies = ToolStrategies() | ||
| self._logger = logging.getLogger(__name__) | ||
|
|
||
| async def mutate(self, tool: dict[str, Any], phase: str) -> dict[str, Any]: | ||
| """Generate fuzzed arguments for a tool.""" | ||
| return await self.strategies.fuzz_tool_arguments(tool, phase) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align ToolMutator and ResultCollector docs with current implementations
There are a couple of spots where the doc no longer matches the code:
-
ToolMutator snippet: here it subclasses
BaseMutatorand keeps a_logger, whereas the concrete implementation inmcp_fuzzer/fuzz_engine/mutators/tool_mutator.pysubclassesMutatorand currently has no logger. Either update the example to mirror the real class or adjust the implementation to match the documented base/attributes. -
ResultCollector snippet: the doc describes a stateful collector with
self.results/self.errorsand afilter_results(filter_type="all")API, but the implementation inmcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py(and the new tests) expose a stateless interface:def collect_results(self, batch_results: dict[str, list[Any]]) -> list[dict[str, Any]]: ... def filter_results(self, results: list[dict[str, Any]], success_only: bool = False) -> list[dict[str, Any]]: ...
It would be good to update this section so the architecture doc reflects the actual public surface.
Also applies to: 374-395
🤖 Prompt for AI Agents
In docs/architecture/fuzz-engine.md around lines 109 to 124 (and similarly
update lines 374 to 395), the doc examples for ToolMutator and ResultCollector
are out of sync with the code; update the ToolMutator snippet to mirror the real
implementation in mcp_fuzzer/fuzz_engine/mutators/tool_mutator.py by subclassing
Mutator (not BaseMutator) and removing the unused _logger and adjust the async
mutate signature to match the concrete class, and revise the ResultCollector
section to describe the stateless API used in
mcp_fuzzer/fuzz_engine/fuzzerreporter/collector.py (show
collect_results(batch_results: dict[str, list[Any]]) -> list[dict[str, Any]] and
filter_results(results: list[dict[str, Any]], success_only: bool = False) ->
list[dict[str, Any]] with a brief note on their behavior) so the architecture
doc reflects the actual public surface.
|
|
||
| - **Runtime Management** - Async process management and monitoring | ||
|
|
||
| - **CLI Interface** - User-friendly command-line interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Minor wording nit: “CLI Interface” tautology
“CLI Interface – User-friendly command-line interface” is slightly redundant (“I” is already “Interface”). Consider simplifying to “CLI – User-friendly command-line interface” or “Command-line interface (CLI) – …”.
🧰 Tools
🪛 LanguageTool
[style] ~109-~109: This phrase is redundant (‘I’ stands for ‘Interface’). Use simply “CLIInterface”.
Context: ... process management and monitoring - CLI Interface - User-friendly command-line interfac...
(ACRONYM_TAUTOLOGY)
🤖 Prompt for AI Agents
In docs/index.md around line 109, the heading "CLI Interface - User-friendly
command-line interface" uses a redundant phrase ("CLI" already means
"command-line interface"); update the text to remove the tautology by either
using "CLI - User-friendly command-line interface" or "Command-line interface
(CLI) - User-friendly command-line interface" so the wording is concise and
non-redundant.
| ### Getting Started | ||
| - **[Getting Started](getting-started/getting-started.md)** - Installation and basic usage | ||
| - **[Configuration](configuration/configuration.md)** - Configuration options and file formats (YAML/TOML) | ||
| - **[Architecture](architecture/architecture.md)** - System design and components | ||
| - **[Runtime Management](components/runtime-management.md)** - Process management, watchdog system, and async executor | ||
| - **[Process Management Guide](components/process-management-guide.md)** - Process management best practices and troubleshooting | ||
| - **[Client Architecture](architecture/client-architecture.md)** - Client package structure | ||
| - **[Examples](getting-started/examples.md)** - Working examples and configurations | ||
| - **[Reference](development/reference.md)** - Complete API reference | ||
|
|
||
| ### Architecture | ||
| - **[Architecture Overview](architecture/architecture.md)** - System design and components | ||
| - **[Fuzz Engine](architecture/fuzz-engine.md)** - Detailed fuzz engine design (Mutators, Executor, FuzzerReporter) | ||
| - **[Client Architecture](architecture/client-architecture.md)** - Client package structure | ||
| - **[Async Executor](architecture/async-executor.md)** - Async execution framework | ||
|
|
||
| ### Configuration | ||
| - **[Configuration](configuration/configuration.md)** - Configuration options and file formats (YAML/TOML) | ||
| - **[Network Policy](configuration/network-policy.md)** - Network access control | ||
|
|
||
| ### Components | ||
| - **[Runtime Management](components/runtime-management.md)** - Process management, watchdog system | ||
| - **[Process Management Guide](components/process-management-guide.md)** - Process management best practices | ||
| - **[Safety Guide](components/safety.md)** - Safety system configuration | ||
|
|
||
| ### Development | ||
| - **[Reference](development/reference.md)** - Complete API reference | ||
| - **[Exceptions](development/exceptions.md)** - Error handling and exception hierarchy | ||
| - **[Contributing](development/contributing.md)** - Development and contribution guide | ||
|
|
||
| ### Testing | ||
| - **[Fuzz Results](testing/fuzz-results.md)** - Latest fuzzing test results | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add blank lines after ### headings to satisfy markdownlint (MD022)
Headings like ### Getting Started, ### Architecture, ### Configuration, ### Components, ### Development, and ### Testing are immediately followed by list items without a blank line. markdownlint (MD022) is already flagging these.
Insert a blank line after each of these headings, e.g.:
-### Getting Started
-- **[Getting Started](getting-started/getting-started.md)** ...
+### Getting Started
+
+- **[Getting Started](getting-started/getting-started.md)** ...Repeat for the other affected subsections.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Getting Started | |
| - **[Getting Started](getting-started/getting-started.md)** - Installation and basic usage | |
| - **[Configuration](configuration/configuration.md)** - Configuration options and file formats (YAML/TOML) | |
| - **[Architecture](architecture/architecture.md)** - System design and components | |
| - **[Runtime Management](components/runtime-management.md)** - Process management, watchdog system, and async executor | |
| - **[Process Management Guide](components/process-management-guide.md)** - Process management best practices and troubleshooting | |
| - **[Client Architecture](architecture/client-architecture.md)** - Client package structure | |
| - **[Examples](getting-started/examples.md)** - Working examples and configurations | |
| - **[Reference](development/reference.md)** - Complete API reference | |
| ### Architecture | |
| - **[Architecture Overview](architecture/architecture.md)** - System design and components | |
| - **[Fuzz Engine](architecture/fuzz-engine.md)** - Detailed fuzz engine design (Mutators, Executor, FuzzerReporter) | |
| - **[Client Architecture](architecture/client-architecture.md)** - Client package structure | |
| - **[Async Executor](architecture/async-executor.md)** - Async execution framework | |
| ### Configuration | |
| - **[Configuration](configuration/configuration.md)** - Configuration options and file formats (YAML/TOML) | |
| - **[Network Policy](configuration/network-policy.md)** - Network access control | |
| ### Components | |
| - **[Runtime Management](components/runtime-management.md)** - Process management, watchdog system | |
| - **[Process Management Guide](components/process-management-guide.md)** - Process management best practices | |
| - **[Safety Guide](components/safety.md)** - Safety system configuration | |
| ### Development | |
| - **[Reference](development/reference.md)** - Complete API reference | |
| - **[Exceptions](development/exceptions.md)** - Error handling and exception hierarchy | |
| - **[Contributing](development/contributing.md)** - Development and contribution guide | |
| ### Testing | |
| - **[Fuzz Results](testing/fuzz-results.md)** - Latest fuzzing test results | |
| ### Getting Started | |
| - **[Getting Started](getting-started/getting-started.md)** - Installation and basic usage | |
| - **[Examples](getting-started/examples.md)** - Working examples and configurations | |
| ### Architecture | |
| - **[Architecture Overview](architecture/architecture.md)** - System design and components | |
| - **[Fuzz Engine](architecture/fuzz-engine.md)** - Detailed fuzz engine design (Mutators, Executor, FuzzerReporter) | |
| - **[Client Architecture](architecture/client-architecture.md)** - Client package structure | |
| - **[Async Executor](architecture/async-executor.md)** - Async execution framework | |
| ### Configuration | |
| - **[Configuration](configuration/configuration.md)** - Configuration options and file formats (YAML/TOML) | |
| - **[Network Policy](configuration/network-policy.md)** - Network access control | |
| ### Components | |
| - **[Runtime Management](components/runtime-management.md)** - Process management, watchdog system | |
| - **[Process Management Guide](components/process-management-guide.md)** - Process management best practices | |
| - **[Safety Guide](components/safety.md)** - Safety system configuration | |
| ### Development | |
| - **[Reference](development/reference.md)** - Complete API reference | |
| - **[Exceptions](development/exceptions.md)** - Error handling and exception hierarchy | |
| - **[Contributing](development/contributing.md)** - Development and contribution guide | |
| ### Testing | |
| - **[Fuzz Results](testing/fuzz-results.md)** - Latest fuzzing test results |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
115-115: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
119-119: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
125-125: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
129-129: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
134-134: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
139-139: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In docs/index.md around lines 115 to 141, several level-3 headings (### Getting
Started, ### Architecture, ### Configuration, ### Components, ### Development,
### Testing) are followed immediately by list items which violates markdownlint
MD022; fix this by inserting a single blank line after each of those headings so
each heading is separated from the following list item (repeat for every
affected subsection in that range).
| from .fuzz_engine import ( | ||
| ToolMutator, | ||
| ProtocolMutator, | ||
| BatchMutator, | ||
| ToolExecutor, | ||
| ProtocolExecutor, | ||
| BatchExecutor, | ||
| ProtocolStrategies, | ||
| ToolStrategies, | ||
| ) | ||
|
|
||
| __version__ = "0.1.9" | ||
| __all__ = [ | ||
| "ToolFuzzer", | ||
| "ProtocolFuzzer", | ||
| "ToolMutator", | ||
| "ProtocolMutator", | ||
| "BatchMutator", | ||
| "ToolExecutor", | ||
| "ProtocolExecutor", | ||
| "BatchExecutor", | ||
| "ToolStrategies", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Top-level exports align with new Mutators/Executors architecture
Exposing ToolMutator, ProtocolMutator, BatchMutator, and the corresponding *Executor classes (plus strategies) via mcp_fuzzer matches the refactored design and keeps the public surface coherent.
If you expect external users to rely heavily on reporting/metrics, you might later consider also re-exporting the fuzzerreporter components (e.g., ResultBuilder, ResultCollector, MetricsCalculator) here, but the current surface is internally consistent.
🤖 Prompt for AI Agents
In mcp_fuzzer/__init__.py around lines 17 to 36, the module imports
ProtocolStrategies but fails to export it in the __all__ list; update the
__all__ list to include "ProtocolStrategies" (keeping the existing entries
intact) so the public API matches the imported symbols.
| self.protocol_mutator = ProtocolMutator() | ||
| # Use ProtocolExecutor to get PROTOCOL_TYPES | ||
| self._protocol_executor = ProtocolExecutor( | ||
| None, max_concurrency=max_concurrency | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider avoiding ProtocolExecutor instantiation when only PROTOCOL_TYPES are needed
ProtocolClient creates a ProtocolExecutor with transport=None and later uses it only to:
- Read
PROTOCOL_TYPESin_get_protocol_types, and - Call
shutdown()inProtocolClient.shutdown.
This also constructs an AsyncFuzzExecutor instance that the client never uses for execution.
If you want to reduce overhead and coupling here, you could:
- Access the class-level constant directly, and
- Drop the unused executor instance (and its shutdown):
-from ..fuzz_engine.executor import ProtocolExecutor
+from ..fuzz_engine.executor import ProtocolExecutor
@@
- # Use ProtocolExecutor to get PROTOCOL_TYPES
- self._protocol_executor = ProtocolExecutor(
- None, max_concurrency=max_concurrency
- )
@@
- # The protocol executor knows which protocol types to fuzz
- return list(getattr(self._protocol_executor, "PROTOCOL_TYPES", ()))
+ # Use the class-level protocol type list from ProtocolExecutor
+ return list(getattr(ProtocolExecutor, "PROTOCOL_TYPES", ()))
@@
- async def shutdown(self) -> None:
- """Shutdown the protocol client."""
- await self._protocol_executor.shutdown()
+ async def shutdown(self) -> None:
+ """Shutdown the protocol client."""
+ # No executor resources owned here; nothing to shut down yet.
+ return NoneFunctionality stays the same while avoiding an otherwise-unused executor instance.
Also applies to: 214-247, 387-389
🤖 Prompt for AI Agents
In mcp_fuzzer/client/protocol_client.py around lines 39 to 43 (and similarly at
214-247, 387-389), the class instantiates ProtocolExecutor(None, ...) only to
read PROTOCOL_TYPES and later call shutdown, which needlessly constructs an
AsyncFuzzExecutor; remove the unused ProtocolExecutor instance, replace reads of
PROTOCOL_TYPES with direct access to ProtocolExecutor.PROTOCOL_TYPES (or move
PROTOCOL_TYPES to a module/class-level constant if preferred), and remove the
corresponding shutdown call and any references to self._protocol_executor so the
client no longer creates or shuts down an unused executor.
| def test_string_with_exclusive_minimum(self): | ||
| """Test string generation with exclusiveMinimum length.""" | ||
| schema = {"type": "string", "minLength": 5, "exclusiveMinimum": 10} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
|
|
||
| def test_number_with_exclusive_minimum(self): | ||
| """Test number generation with exclusiveMinimum.""" | ||
| schema = {"type": "number", "exclusiveMinimum": 10.0, "maximum": 20.0} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, (int, float)) | ||
| self.assertGreater(result, 10.0) | ||
| self.assertLessEqual(result, 20.0) | ||
|
|
||
| def test_number_with_exclusive_maximum(self): | ||
| """Test number generation with exclusiveMaximum.""" | ||
| schema = {"type": "number", "minimum": 10.0, "exclusiveMaximum": 20.0} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, (int, float)) | ||
| self.assertGreaterEqual(result, 10.0) | ||
| self.assertLess(result, 20.0) | ||
|
|
||
| def test_number_with_multiple_of(self): | ||
| """Test number generation with multipleOf constraint.""" | ||
| schema = {"type": "number", "multipleOf": 5, "minimum": 10, "maximum": 50} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, (int, float)) | ||
| self.assertEqual(result % 5, 0) | ||
|
|
||
| def test_integer_with_multiple_of(self): | ||
| """Test integer generation with multipleOf constraint.""" | ||
| schema = {"type": "integer", "multipleOf": 3, "minimum": 10, "maximum": 30} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, int) | ||
| self.assertEqual(result % 3, 0) | ||
|
|
||
| def test_string_with_pattern(self): | ||
| """Test string generation with pattern.""" | ||
| schema = {"type": "string", "pattern": "^[a-z]+$"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
|
|
||
| def test_string_format_date(self): | ||
| """Test string generation with date format.""" | ||
| schema = {"type": "string", "format": "date"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
| # Date format should produce a string (format may vary) | ||
| self.assertGreater(len(result), 0) | ||
|
|
||
| def test_string_format_datetime(self): | ||
| """Test string generation with date-time format.""" | ||
| schema = {"type": "string", "format": "date-time"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
| self.assertIn("T", result) | ||
|
|
||
| def test_string_format_time(self): | ||
| """Test string generation with time format.""" | ||
| schema = {"type": "string", "format": "time"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
| # Should produce a string (format implementation may vary) | ||
| self.assertGreater(len(result), 0) | ||
|
|
||
| def test_string_format_uuid(self): | ||
| """Test string generation with UUID format.""" | ||
| schema = {"type": "string", "format": "uuid"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
| # Should produce a string (format implementation may vary) | ||
| self.assertGreater(len(result), 0) | ||
|
|
||
| def test_string_format_ipv4(self): | ||
| """Test string generation with ipv4 format.""" | ||
| schema = {"type": "string", "format": "ipv4"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
| # Should produce a string (format implementation may vary) | ||
| self.assertGreater(len(result), 0) | ||
|
|
||
| def test_string_format_ipv6(self): | ||
| """Test string generation with ipv6 format.""" | ||
| schema = {"type": "string", "format": "ipv6"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, str) | ||
| # Should produce a string (format implementation may vary) | ||
| self.assertGreater(len(result), 0) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Clarify intent and strengthen some format/pattern/“exclusive” tests
A few tests in this block are effectively smoke tests but their docstrings suggest more specific behavior:
test_string_with_exclusive_minimum(Lines 93-98) mentions “exclusiveMinimum length” butexclusiveMinimumis not a standard string constraint and_handle_string_typeonly honorsminLength/maxLength/pattern/format. If the goal is just “doesn’t crash when unknown numeric constraints are present”, consider adjusting the docstring, or alternatively assert something aboutlen(result)relative tominLength.test_string_with_pattern(Lines 129-134) only checks that the result is astr. If you want to validate pattern support, you could also assert thatre.fullmatch("^[a-z]+$", result)succeeds (with a fallback comment acknowledging possible generator fallbacks).- The various
formattests (date, time, uuid, ipv4/ipv6, hostname) intentionally stay loose, which is fine; just ensure the docstrings reflect that you only verify “non-empty string” (plus the"T"check fordate-time), not strict RFC conformance.
Not strictly required, but tightening where feasible will improve the regression signal from these “advanced” tests.
🤖 Prompt for AI Agents
tests/unit/fuzz_engine/mutators/test_schema_parser_advanced.py lines 93-181:
several tests claim stricter behavior than they assert; update them to either
clarify intent in the docstrings or add stronger assertions: for
test_string_with_exclusive_minimum, change the docstring to state “ensures
unknown numeric constraints don’t crash and minLength honored” and add an
assertion that len(result) >= schema["minLength"]; for test_string_with_pattern,
assert the pattern actually matches (e.g. use re.fullmatch("^[a-z]+$", result))
with a comment that generators may fallback and the assertion can be relaxed if
non-deterministic; for the format tests (date, date-time, time, uuid, ipv4,
ipv6) update docstrings to explicitly say “verifies non-empty string (and for
date-time, contains 'T')” or, where feasible, add lightweight format checks
(e.g. presence of '-' for date) while keeping tests tolerant of generator
variations.
| def test_array_with_unique_items(self): | ||
| """Test array generation with uniqueItems constraint.""" | ||
| schema = { | ||
| "type": "array", | ||
| "items": {"type": "integer"}, | ||
| "minItems": 3, | ||
| "uniqueItems": True, | ||
| } | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, list) | ||
| # Check uniqueness | ||
| self.assertEqual(len(result), len(set(result))) | ||
|
|
||
| def test_object_with_additional_properties(self): | ||
| """Test object generation with additionalProperties.""" | ||
| schema = { | ||
| "type": "object", | ||
| "properties": {"name": {"type": "string"}}, | ||
| "additionalProperties": {"type": "integer"}, | ||
| } | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, dict) | ||
|
|
||
| def test_object_without_properties(self): | ||
| """Test object generation without properties definition.""" | ||
| schema = {"type": "object", "minProperties": 2, "maxProperties": 5} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, dict) | ||
| self.assertGreaterEqual(len(result), 2) | ||
| self.assertLessEqual(len(result), 5) | ||
|
|
||
| def test_deep_recursion_limit(self): | ||
| """Test that deep recursion is limited.""" | ||
| schema = { | ||
| "type": "object", | ||
| "properties": { | ||
| "nested": { | ||
| "type": "object", | ||
| "properties": { | ||
| "nested": { | ||
| "type": "object", | ||
| "properties": { | ||
| "nested": { | ||
| "type": "object", | ||
| "properties": { | ||
| "nested": {"type": "object"}, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, dict) | ||
|
|
||
| def test_const_value(self): | ||
| """Test schema with const value.""" | ||
| schema = {"const": "fixed_value"} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertEqual(result, "fixed_value") | ||
|
|
||
| def test_const_value_aggressive(self): | ||
| """Test string generation with const value in aggressive mode.""" | ||
| schema = {"const": 42} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="aggressive") | ||
| # In aggressive mode, can return any type - just check it returns something | ||
| # (could be the const value, or an edge case type) | ||
| self.assertTrue( | ||
| result is not None or result == 0 or result == [] or result == "" | ||
| ) | ||
|
|
||
| def test_oneOf_selection(self): | ||
| """Test oneOf schema combination.""" | ||
| schema = { | ||
| "oneOf": [ | ||
| {"type": "string", "minLength": 5}, | ||
| {"type": "integer", "minimum": 10}, | ||
| {"type": "boolean"}, | ||
| ] | ||
| } | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertTrue( | ||
| isinstance(result, str) | ||
| or isinstance(result, int) | ||
| or isinstance(result, bool) | ||
| ) | ||
|
|
||
| def test_anyOf_selection(self): | ||
| """Test anyOf schema combination.""" | ||
| schema = { | ||
| "anyOf": [ | ||
| {"type": "string", "minLength": 5}, | ||
| {"type": "integer", "minimum": 10}, | ||
| ] | ||
| } | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertTrue(isinstance(result, str) or isinstance(result, int)) | ||
|
|
||
| def test_allOf_with_nested_properties(self): | ||
| """Test allOf with nested object properties.""" | ||
| schema = { | ||
| "allOf": [ | ||
| { | ||
| "type": "object", | ||
| "properties": {"name": {"type": "string", "minLength": 3}}, | ||
| "required": ["name"], | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "properties": {"age": {"type": "integer", "minimum": 0}}, | ||
| }, | ||
| ] | ||
| } | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertIsInstance(result, dict) | ||
| self.assertIn("name", result) | ||
|
|
||
| def test_aggressive_mode_violations(self): | ||
| """Test aggressive mode generates boundary violations.""" | ||
| schema = {"type": "string", "minLength": 5, "maxLength": 10} | ||
| # Run multiple times to potentially hit boundary violations | ||
| results = [ | ||
| make_fuzz_strategy_from_jsonschema(schema, phase="aggressive") | ||
| for _ in range(20) | ||
| ] | ||
| # At least some should be strings (even in aggressive mode) | ||
| string_results = [r for r in results if isinstance(r, str)] | ||
| self.assertGreater(len(string_results), 0) | ||
|
|
||
| def test_aggressive_mode_type_violations(self): | ||
| """Test aggressive mode can generate wrong types.""" | ||
| schema = {"type": "integer", "minimum": 10, "maximum": 20} | ||
| results = [ | ||
| make_fuzz_strategy_from_jsonschema(schema, phase="aggressive") | ||
| for _ in range(20) | ||
| ] | ||
| # Should have mixed types or boundary violations | ||
| self.assertGreater(len(results), 0) | ||
|
|
||
| def test_empty_schema(self): | ||
| """Test handling of empty schema.""" | ||
| schema = {} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| # Should return some value | ||
| self.assertIsNot(result, None) | ||
|
|
||
| def test_type_array_multiple_types(self): | ||
| """Test schema with array of types.""" | ||
| schema = {"type": ["string", "integer", "null"]} | ||
| result = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
| self.assertTrue( | ||
| isinstance(result, str) or isinstance(result, int) or result is None | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Several “aggressive/empty/mixed-type” tests have very weak assertions
In this section, a number of tests mostly check “does not crash” while their names/docstrings imply stronger guarantees:
test_const_value_aggressive(Lines 252-260):assertTrue(result is not None or result == 0 or result == [] or result == "")is equivalent to “result is not None” because any0/[]/""already satisfyresult is not None. If the requirement is simply “returns something”,self.assertIsNotNone(result)would be clearer; if you want to admitNoneas a valid edge case, the assertion should change accordingly.
test_aggressive_mode_violations(Lines 308-319):- Docstring says “generates boundary violations”, but the only check is that at least one result is a
str. If the intent is to verify that some values violateminLength/maxLengthor even the"type"constraint, consider asserting that some outputs fall outside the specified bounds or differ in type.
- Docstring says “generates boundary violations”, but the only check is that at least one result is a
test_aggressive_mode_type_violations(Lines 320-329):- Similarly, the test name/docstring talk about “type violations” but the assertion only checks
len(results) > 0, which can never fail. If aggressive mode is expected to sometimes emit non-intvalues or out-of-range integers, asserting for the presence of at least one such violation would better capture that contract.
- Similarly, the test name/docstring talk about “type violations” but the assertion only checks
test_empty_schema(Lines 330-336):- Currently just asserts the result is not
None. If the default behavior for{}is more specific (e.g., a particular default type or set of candidate values), tightening this would give stronger coverage; if it truly is “any non-None value”, the docstring might call that out.
- Currently just asserts the result is not
These aren’t correctness bugs, but aligning the assertions with the test names/docstrings would make this advanced suite much more effective.
🤖 Prompt for AI Agents
In tests/unit/fuzz_engine/mutators/test_schema_parser_advanced.py around lines
189-344, several tests use weak or vacuous assertions that don't match their
docstrings: replace the tautological assert in test_const_value_aggressive
(lines ~252-260) with self.assertIsNotNone(result) or adjust docstring to allow
None; in test_aggressive_mode_violations (lines ~308-319) generate multiple
results and assert at least one violates the minLength/maxLength (e.g., len(s) <
minLength or len(s) > maxLength) or has wrong type; in
test_aggressive_mode_type_violations (lines ~320-329) assert that at least one
result is not an int or is out of the specified range to prove type/boundary
violations; for test_empty_schema (lines ~330-336) either tighten the
expectation to the known default behavior or explicitly document and assert that
any non-None value is acceptable using self.assertIsNotNone(result).
| def test_handle_integer_type_edge_cases(self): | ||
| """Test _handle_integer_type with edge case constraints.""" | ||
| schema = { | ||
| "type": "integer", | ||
| "minimum": 100, | ||
| "maximum": 100, # min == max | ||
| } | ||
| result = _handle_integer_type(schema, phase="realistic") | ||
| self.assertEqual(result, 100) | ||
|
|
||
| def test_handle_number_type_edge_cases(self): | ||
| """Test _handle_number_type with edge case constraints.""" | ||
| schema = { | ||
| "type": "number", | ||
| "minimum": 5.5, | ||
| "maximum": 5.5, # min == max | ||
| } | ||
| result = _handle_number_type(schema, phase="realistic") | ||
| self.assertEqual(result, 5.5) | ||
|
|
||
| def test_handle_array_type_empty_allowed(self): | ||
| """Test _handle_array_type allows empty arrays.""" | ||
| schema = {"type": "array", "items": {"type": "string"}, "minItems": 0} | ||
| result = _handle_array_type(schema, phase="realistic", recursion_depth=0) | ||
| self.assertIsInstance(result, list) | ||
|
|
||
| def test_handle_object_type_no_required(self): | ||
| """Test _handle_object_type without required fields.""" | ||
| schema = { | ||
| "type": "object", | ||
| "properties": { | ||
| "optional1": {"type": "string"}, | ||
| "optional2": {"type": "integer"}, | ||
| }, | ||
| } | ||
| result = _handle_object_type(schema, phase="realistic", recursion_depth=0) | ||
| self.assertIsInstance(result, dict) | ||
|
|
||
| def test_handle_string_type_aggressive_boundaries(self): | ||
| """Test _handle_string_type in aggressive mode.""" | ||
| schema = {"type": "string", "minLength": 5, "maxLength": 10} | ||
| results = [_handle_string_type(schema, phase="aggressive") for _ in range(20)] | ||
| # Should have variety | ||
| self.assertGreater(len(results), 0) | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Edge-case handler tests are good; consider small assertion upgrades
The edge tests for _handle_integer_type, _handle_number_type, _handle_array_type, _handle_object_type, and _handle_string_type are directionally correct:
test_handle_integer_type_edge_casesandtest_handle_number_type_edge_cases(Lines 345-364) nicely pin min==max behavior.test_handle_array_type_empty_allowedandtest_handle_object_type_no_required(Lines 365-382) ensure permissive schemas don’t crash.
For test_handle_string_type_aggressive_boundaries (Lines 383-388), you currently only assert that results is non-empty. If the intent is to verify variety or boundary exploration in aggressive mode, you could cheaply strengthen this by e.g.:
- Asserting that at least one result is outside
[minLength, maxLength]or that multiple distinct strings are produced.
Not mandatory, but would better validate the “aggressive” behavior.
🤖 Prompt for AI Agents
In tests/unit/fuzz_engine/mutators/test_schema_parser_advanced.py around lines
383 to 388, strengthen test_handle_string_type_aggressive_boundaries by
asserting that the aggressive phase produces varied outputs: collect the 20
generated strings, assert there are at least 2 distinct values, and additionally
assert that at least one result has length outside the inclusive [minLength,
maxLength] range (or at least one shorter or longer) to validate boundary
exploration; update assertions accordingly while keeping the test deterministic
enough (e.g., check distinct count and existence of any out-of-range length).
|
|
||
| import unittest | ||
| import pytest | ||
| from unittest.mock import MagicMock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Unused import: MagicMock.
MagicMock is imported but not used in any test method in this file.
-from unittest.mock import MagicMock📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from unittest.mock import MagicMock |
🤖 Prompt for AI Agents
In tests/unit/fuzz_engine/strategy/test_strategy_manager_tool.py around line 8,
the import of MagicMock is unused; remove the "from unittest.mock import
MagicMock" import or replace it with the correct mock import used in the file.
Ensure there are no remaining references to MagicMock after removal and run
tests to confirm no import-related failures.
Fuzz Engine Redesign - Comprehensive Pull Request Summary
Executive Summary
This PR implements a comprehensive architectural redesign of the MCP Server Fuzzer's core fuzzing engine, transforming it from a monolithic, mixed-responsibility design into a clean, modular three-module architecture. The redesign improves maintainability, testability, extensibility, and code quality while maintaining backward compatibility through a new public API.
Architecture Transformation
Previous Architecture
ToolFuzzerandProtocolFuzzercombined data generation, execution, result tracking, and reportingfuzz_engine/fuzzer/module with sprawling logicNew Three-Module Architecture
1. Mutators Module (
fuzz_engine/mutators/)Mutatorclass defining the mutation interfaceToolMutator- generates fuzzed tool arguments viaToolStrategiesProtocolMutator- generates fuzzed protocol messages viaProtocolStrategiesBatchMutator- generates batch request payloadsmutators/strategies/(previously at top-level)2. Executor Module (
fuzz_engine/executor/)ToolExecutor- orchestrates tool fuzzing with safety integrationProtocolExecutor- orchestrates protocol fuzzing with invariant checkingBatchExecutor- orchestrates batch fuzzingAsyncFuzzExecutor- provides async execution framework with bounded concurrencyinvariantssubmodule (relocated fromfuzz_engine/invariants.py)3. FuzzerReporter Module (
fuzz_engine/fuzzerreporter/)ResultBuilder- standardizes result structure across tool/protocol/batchResultCollector- aggregates results from batch executionMetricsCalculator- computes success rates and rejection metricsDesign Patterns Applied
1. Separation of Concerns ✓ (IMPROVED)
Clear, single responsibility for each component:
Before:
ToolFuzzerhandled all three concernsAfter: Each component has one clear responsibility
2. Dependency Injection Pattern ✓ (NEW)
All executors accept optional injectable components with sensible defaults:
Benefits:
3. Strategy Pattern ✓ (RETAINED & ENHANCED)
mutators/strategies/4. Builder Pattern ✓ (NEW)
ResultBuilderconstructs standardized result objects:build_tool_result()- for tool fuzzingbuild_protocol_result()- for protocol fuzzingbuild_batch_result()- for batch fuzzing5. Facade Pattern ✓ (MODULE LEVEL)
Public API modules aggregate and re-export components:
mcp_fuzzer.fuzz_engine.mutators- aggregates all mutatorsmcp_fuzzer.fuzz_engine.executor- aggregates all executorsmcp_fuzzer.fuzz_engine.fuzzerreporter- aggregates reportersmcp_fuzzermodule exposes main API6. Factory-like Defaults ✓
Executors instantiate default components if not provided:
SOLID Principles Compliance
S - Single Responsibility Principle ✓ SIGNIFICANTLY IMPROVED
Before:
ToolFuzzer- 200+ lines handling generation, execution, safety, results, phase managementAfter: Each class has one responsibility:
ToolMutator- Generates fuzzed arguments (delegates to strategies)ToolExecutor- Orchestrates execution with safety integrationResultBuilder- Structures results onlyResultCollector- Aggregates results onlyMetricsCalculator- Computes metrics onlyAsyncFuzzExecutor- Handles async concurrency onlyO - Open/Closed Principle ✓ SIGNIFICANTLY IMPROVED
Before: Adding new strategies required modifying executor classes
After:
strategies/without modifying executorsMutatorbase class enables extension without modificationL - Liskov Substitution Principle ✓ IMPROVED
MutatorABC with concrete implementations (Tool, Protocol, Batch)mutate()interfaceI - Interface Segregation Principle ✓ SIGNIFICANTLY IMPROVED
Before: Single large
ToolFuzzerinterface with mixed concernsAfter: Focused, segregated interfaces:
Mutator.mutate()- single method contractResultBuildermethods are separate (tool, protocol, batch specific)execute,execute_both_phases,execute_multiple)D - Dependency Inversion Principle ✓ SIGNIFICANTLY IMPROVED
Code Quality and Design Smells Analysis
✓ Strong Practices Implemented
Type Safety: Comprehensive type hints throughout
FuzzDataResultTypedDict for resultsAsynchronous by Default: Proper async/await patterns
AsyncFuzzExecutorwith bounded concurrencyComposition Over Inheritance:
Testability: Designed for comprehensive testing
Documentation: Extensive architecture documentation
docs/architecture/fuzz-engine.md- 750+ linesRuntime Module: Separate concerns for process lifecycle
ProcessManager,ProcessWatchdogfor runtime management⚠ Minor Concerns & Mitigation
Optional Component Defaults in Constructors
Boolean Parameter for Safety (
enable_safety)enable_safety: bool = TrueinToolExecutorInconsistent Result Types
FuzzDataResulttyped objectsInvariants Module Relocation
fuzz_engine/invariants.py→fuzz_engine/executor/invariants.pyStrategy-to-Mutator Delegation
Breaking Changes & Migration
Public API Changes
ToolFuzzer→ ✓ToolExecutor+ToolMutatorProtocolFuzzer→ ✓ProtocolExecutor+ProtocolMutatorfuzz_tool()→ ✓execute()fuzz_protocol_type()→ ✓execute()fuzz_engine.fuzzer→ ✓fuzz_engine.executor+fuzz_engine.mutatorsMigration Path (Documented)
Strengths of Redesign
Test Coverage & Validation
pytestmarkers for organizing tests (unit,fuzz_engine,executor, etc.)Public API Summary
Top-Level Exports (
mcp_fuzzer/__init__.py)Conclusion
This redesign successfully applies SOLID principles and established design patterns to create a more maintainable, testable, and extensible fuzzing engine. The shift from monolithic fuzzers to modular components with clear boundaries represents a mature architectural evolution that eliminates technical debt, improves code organization, and sets a foundation for future enhancements. The comprehensive test coverage and extensive documentation demonstrate commitment to quality and ease of adoption.