Add JSON Schema parser and property-based invariants system #75
Conversation
…types, and array/object constraints
- Implement comprehensive JSON Schema parser with support for types, constraints, and combinations - Add property-based invariants for response validation and error checking - Update documentation with schema parser and invariants details - Fix tests and linting issues Fixes #10: Add property-based invariants and checks Fixes #12: Enhance JSON Schema parsing for fuzz strategies Fixes #53: Type safety and code quality enhancements
WalkthroughAdds a JSON Schema–driven schema_parser and a new invariants module, integrates invariant verification into protocol_fuzzer results with batch/timeouts, updates realistic/aggressive tool strategies to prefer the parser with fallbacks, and adds/updates documentation and tests for schema parsing and invariants. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester
participant ProtocolFuzzer
participant Server
participant Invariants
Tester->>ProtocolFuzzer: Run fuzz (request, generate_only=false)
ProtocolFuzzer->>Server: Send fuzzed request
Server-->>ProtocolFuzzer: Response (if any)
alt Response exists and not generate_only
ProtocolFuzzer->>Invariants: verify_response_invariants(response, expected_codes?, schema?)
Note right of Invariants: may be sync (single) or async batch
Invariants-->>ProtocolFuzzer: ok / raise InvariantViolation / error map
alt Violations found
ProtocolFuzzer->>ProtocolFuzzer: collect & log violations
Note right of ProtocolFuzzer: attach `invariant_violations` to result
end
else No response or generate_only
Note right of ProtocolFuzzer: skip invariant verification
end
ProtocolFuzzer-->>Tester: Fuzz result (includes `invariant_violations`)
sequenceDiagram
autonumber
participant Caller
participant Strategy as ToolStrategy
participant Parser as schema_parser
Caller->>Strategy: fuzz_tool_arguments_...(tool)
Strategy->>Parser: make_fuzz_strategy_from_jsonschema(inputSchema, phase)
Parser-->>Strategy: args (dict | empty | non-dict)
alt args is dict and non-empty
Strategy->>Caller: return args (may add extras in aggressive)
else Fallback
Strategy->>Strategy: per-property probabilistic generation (80% include)
Strategy->>Caller: return normalized args
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Agent-Hellboy
changed the title
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. |
Agent-Hellboy
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
==========================================
- Coverage 60.83% 59.57% -1.26%
==========================================
Files 51 53 +2
Lines 3679 4126 +447
Branches 542 672 +130
==========================================
+ Hits 2238 2458 +220
- Misses 1299 1477 +178
- Partials 142 191 +49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture.md (1)
321-347: Fix heading numbering (duplicate “6.”).Authentication should be 7; Reporting should be 8.
-### 6. Authentication System +### 7. Authentication System @@ -### 7. Reporting System +### 8. Reporting System
🧹 Nitpick comments (18)
docs/architecture.md (2)
275-288: Tighten grammar and parallelism in Schema Parser bullets.Small wording/punctuation tweaks for consistency and readability.
-**Schema Parser:** +**Schema Parser** @@ -The schema parser provides comprehensive support for parsing JSON Schema definitions and generating appropriate test data based on schema specifications. It handles: +The schema parser parses JSON Schema definitions and generates test data based on schema specifications. It supports: @@ -- Basic types: string, number, integer, boolean, array, object, null -- String constraints: minLength, maxLength, pattern, format -- Number/Integer constraints: minimum, maximum, exclusiveMinimum, exclusiveMaximum, multipleOf -- Array constraints: minItems, maxItems, uniqueItems -- Object constraints: required properties, minProperties, maxProperties -- Schema combinations: oneOf, anyOf, allOf -- Enums and constants +- Basic types: string, number, integer, boolean, array, object, null. +- String constraints: minLength, maxLength, pattern, format. +- Number/integer constraints: minimum, maximum, exclusiveMinimum, exclusiveMaximum, multipleOf. +- Array constraints: minItems, maxItems, uniqueItems. +- Object constraints: required, minProperties, maxProperties. +- Compositions: oneOf, anyOf, allOf. +- Enums and const. @@ -The module supports both "realistic" and "aggressive" fuzzing strategies, where realistic mode generates valid data conforming to the schema, while aggressive mode intentionally generates edge cases and invalid data to test error handling. +Supports both “realistic” (valid/conforming) and “aggressive” (edge cases/invalid) strategies.
289-303: Clarify invariants scope (single vs batch) and JSON-RPC nuance.Mention batch verification availability and that notifications must not yield responses.
-### 5. Invariants System +### 5. Invariants System @@ -These invariants serve as runtime assertions that validate the behavior of the server being tested, helping to identify potential issues that might not be caught by simple error checking. +These invariants serve as runtime assertions that validate server behavior. For JSON‑RPC, responses to notifications are not expected and are treated as violations. Batch responses can be validated with `verify_batch_responses`.mcp_fuzzer/fuzz_engine/invariants.py (2)
158-183: Improve schema validation diagnostics and specificity.Catch jsonschema.ValidationError to surface path/context; optionally enable format checking.
- if HAVE_JSONSCHEMA: + if HAVE_JSONSCHEMA: try: - jsonschema_validate(instance=response, schema=schema) + # If available, prefer explicit validator with format checking + try: + from jsonschema import Draft7Validator, FormatChecker # py3.8+ + Draft7Validator(schema, format_checker=FormatChecker()).validate(response) + except Exception: + # Fallback to generic validate + jsonschema_validate(instance=response, schema=schema) return True - except Exception as e: + except Exception as e: + try: + from jsonschema.exceptions import ValidationError # type: ignore + except Exception: + ValidationError = Exception # best effort raise InvariantViolation( - f"Response does not conform to schema: {e}", response + f"Response does not conform to schema: {e}", response )
247-282: Optional: detect unexpected new keys in after_state.Currently you only ensure keys aren’t removed; consider blocking unexpected additions unless allowed.
# Check that all keys in before_state are also in after_state for key in before_state: if key not in after_state: raise InvariantViolation(f"Key '{key}' missing from after_state") + # Optionally flag unexpected new keys + for key in after_state: + if key not in before_state and key not in (allowed_changes or []): + raise InvariantViolation(f"Unexpected new key in after_state: '{key}'") + # Check that values for keys not in allowed_changes are the samemcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (3)
287-299: Broaden return type for server_response to allow batch arrays.Adjust type hints to match runtime behavior.
- ) -> Tuple[Optional[Dict[str, Any]], Optional[str]]: + ) -> Tuple[Optional[Any], Optional[str]]:
324-326: Align create_fuzz_result typing with potential list responses.If adopting the broader type hint, reflect it here too.
- server_response: Optional[Dict[str, Any]], + server_response: Optional[Any],
340-349: Optional: surface invariants outcome explicitly.Consider adding invariants_ok: bool for quick scanning without parsing messages.
return { "protocol_type": protocol_type, "run": run_index + 1, "fuzz_data": fuzz_data, "success": server_error is None, "server_response": server_response, "server_error": server_error, "server_rejected_input": server_error is not None, "invariant_violations": [], # Will be populated if violations occur + "invariants_ok": True, }And set
result["invariants_ok"] = not invariant_violationswhere you attach violations.tests/test_schema_parser.py (1)
182-202: Inconsistency between minProperties constraint and test assertion.The test only checks the length of the result dictionary but doesn't verify that minProperties is properly enforced. The schema doesn't enforce maxProperties as mentioned in the comment.
Consider updating the test to more accurately reflect the constraints:
def test_make_fuzz_strategy_from_jsonschema_object_constraints(self): """Test BEHAVIOR: respects object constraints.""" - # Object with minProperties and maxProperties + # Object with minProperties constraint object_schema = { "type": "object", "properties": { "prop1": {"type": "string"}, "prop2": {"type": "string"}, "prop3": {"type": "string"}, }, "minProperties": 2, - "maxProperties": 3, } object_result = make_fuzz_strategy_from_jsonschema( object_schema, phase="realistic" ) self.assertIsInstance(object_result, dict, "Should generate a dictionary") self.assertGreaterEqual( len(object_result), 2, "Object should meet minProperties" ) - self.assertLessEqual(len(object_result), 3, "Object should meet maxProperties") + # Note: maxProperties is not enforced by the schema parser implementationmcp_fuzzer/fuzz_engine/strategy/aggressive/tool_strategy.py (1)
256-285: Consider extracting the fallback logic into a separate function.The fallback logic for basic property handling could be extracted into a separate function for better maintainability and testability.
Consider refactoring the fallback logic:
+def _generate_fallback_arguments(properties: Dict[str, Any]) -> Dict[str, Any]: + """Generate fallback arguments when schema parser returns empty result.""" + args = {} + for prop_name, prop_spec in properties.items(): + if random.random() < 0.8: # 80% chance to include each property + prop_type = prop_spec.get("type", "string") + + # Generate aggressive values based on type + if prop_type == "string": + args[prop_name] = generate_aggressive_text() + elif prop_type == "integer": + args[prop_name] = _generate_aggressive_integer() + elif prop_type == "number": + args[prop_name] = _generate_aggressive_float() + elif prop_type == "boolean": + args[prop_name] = random.choice( + [None, "", "true", "false", 0, 1, -1, 42, [], {}, "yes", "no"] + ) + elif prop_type == "array": + args[prop_name] = [ + generate_aggressive_text() for _ in range(random.randint(0, 5)) + ] + elif prop_type == "object": + args[prop_name] = { + generate_aggressive_text(): generate_aggressive_text() + } + else: + args[prop_name] = generate_aggressive_text() + return args def fuzz_tool_arguments_aggressive(tool: Dict[str, Any]) -> Dict[str, Any]: """Generate aggressive/malicious tool arguments.""" from ..schema_parser import make_fuzz_strategy_from_jsonschema schema = tool.get("inputSchema", {}) # Use the enhanced schema parser to generate aggressive values args = make_fuzz_strategy_from_jsonschema(schema, phase="aggressive") # If the schema parser returned something other than a dict, create a default dict if not isinstance(args, dict): args = {} # Ensure we have at least some arguments if not args and schema.get("properties"): # Fallback to basic property handling properties = schema.get("properties", {}) - - for prop_name, prop_spec in properties.items(): - if random.random() < 0.8: # 80% chance to include each property - prop_type = prop_spec.get("type", "string") - - # Generate aggressive values based on type - if prop_type == "string": - args[prop_name] = generate_aggressive_text() - elif prop_type == "integer": - args[prop_name] = _generate_aggressive_integer() - elif prop_type == "number": - args[prop_name] = _generate_aggressive_float() - elif prop_type == "boolean": - args[prop_name] = random.choice( - [None, "", "true", "false", 0, 1, -1, 42, [], {}, "yes", "no"] - ) - elif prop_type == "array": - args[prop_name] = [ - generate_aggressive_text() for _ in range(random.randint(0, 5)) - ] - elif prop_type == "object": - args[prop_name] = { - generate_aggressive_text(): generate_aggressive_text() - } - else: - args[prop_name] = generate_aggressive_text() + args = _generate_fallback_arguments(properties)mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py (4)
78-102: Docstrings mismatch: ISO-8601 “Z” vs “+00:00”timestamp_strings doc says “ending with Z”, but isoformat() yields “+00:00”. Align the doc or append 'Z'.
Apply one of:
- Generate ISO-8601 UTC timestamps ending with Z. + Generate ISO-8601 UTC timestamps in UTC (e.g., 2025-09-02T12:34:56+00:00).Or:
- ).map( - lambda dt: dt.isoformat( - timespec="microseconds" if include_microseconds else "seconds" - ) - ) + ).map( + lambda dt: dt.replace(tzinfo=timezone.utc).isoformat( + timespec="microseconds" if include_microseconds else "seconds" + ).replace("+00:00", "Z") + )
33-42: Remove unused 'alphabet' variable or make it effectivealphabet is computed but never used. Either drop it or honor it to control source bytes.
Minimal cleanup:
- if alphabet is None: - # Use printable ASCII characters for realistic data - alphabet = st.characters( - whitelist_categories=("Lu", "Ll", "Nd", "Pc", "Pd", "Ps", "Pe"), - blacklist_characters="\x00\x01\x02\x03\x04\x05\x06\x07\x08\x0b\x0c", - ) - return st.binary(min_size=min_size, max_size=max_size).map( lambda data: base64.b64encode(data).decode("ascii") )Or make it effective:
- return st.binary(min_size=min_size, max_size=max_size).map( - lambda data: base64.b64encode(data).decode("ascii") - ) + byte_source = ( + st.binary(min_size=min_size, max_size=max_size) + if alphabet is None + else st.text(alphabet=alphabet, min_size=min_size, max_size=max_size).map( + lambda s: s.encode("utf-8") + ) + ) + return byte_source.map(lambda data: base64.b64encode(data).decode("ascii"))
165-186: Defer fallback generation to the schema parser to respect constraintsWhen the parser didn’t populate a property, calling it per-property preserves constraints (e.g., minItems, formats) better than generate_realistic_text().
- if prop_spec.get("type") == "array": + if prop_spec.get("type") == "array": if prop_name not in args or not isinstance(args[prop_name], list): - # Generate a simple array with 1-3 items - if prop_spec.get("items", {}).get("type") == "object": - # For object arrays, create objects with properties - item_props = prop_spec.get("items", {}).get("properties", {}) - items = [] - for _ in range(random.randint(1, 3)): - item = {} - for item_prop, item_spec in item_props.items(): - item[item_prop] = generate_realistic_text() - items.append(item) - args[prop_name] = items - else: - # For simple arrays, generate simple items - args[prop_name] = [ - generate_realistic_text() - for _ in range(random.randint(1, 3)) - ] + args[prop_name] = make_fuzz_strategy_from_jsonschema( + prop_spec, phase="realistic" + )
187-236: Unify format generation with helper strategies to avoid driftFor uuid/date-time/email/uri, prefer reusing uuid_strings()/timestamp_strings() to keep one source of truth.
- elif format_type == "date-time": - args[prop_name] = datetime.now(timezone.utc).isoformat() - elif format_type == "uuid": - args[prop_name] = str(uuid.uuid4()) + elif format_type == "date-time": + args[prop_name] = timestamp_strings().example() + elif format_type == "uuid": + args[prop_name] = uuid_strings().example()mcp_fuzzer/fuzz_engine/strategy/schema_parser.py (1)
79-82: Support 'const' to match docsDocs mention “constants” but code ignores const. Implement trivial handling.
# Handle enums first as they override other type constraints if "enum" in schema and isinstance(schema["enum"], list) and schema["enum"]: return _handle_enum(schema["enum"], phase) + if "const" in schema: + return schema["const"]tests/test_realistic_strategies.py (4)
25-39: Sampling with .example() is fine but can be flaky/slow at scaleConsider using @given with smaller max_examples or a helper loop cap to reduce test time.
71-85: timestamp_strings doc vs behaviorTests parse by replacing 'Z', which hides the doc/code mismatch. Either assert suffix is 'Z' or update the helper as suggested in the strategy file.
201-227: Edge-case tests: add a case for inputSchema=NoneCurrently cover {}, missing key, and empty properties. Add None to catch parser-guard regressions.
# Test with missing inputSchema tool = {} result = fuzz_tool_arguments_realistic(tool) assert result == {} + + # Test with inputSchema=None + tool = {"inputSchema": None} + result = fuzz_tool_arguments_realistic(tool) + assert result == {}
323-402: Strengthen format validations (UUID, date-time) for higher signalQuick wins: parse UUID and fromisoformat to ensure realistic outputs when present.
- if isinstance(result["email_field"], str): + if isinstance(result["email_field"], str): assert "@" in result["email_field"] - if isinstance(result["uri_field"], str): + if isinstance(result["uri_field"], str): assert result["uri_field"].startswith("http") + if isinstance(result.get("uuid_field"), str): + uuid.UUID(result["uuid_field"]) + if isinstance(result.get("datetime_field"), str): + datetime.fromisoformat(result["datetime_field"].replace("Z", "+00:00"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
docs/architecture.md(3 hunks)docs/reference.md(2 hunks)mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py(3 hunks)mcp_fuzzer/fuzz_engine/invariants.py(1 hunks)mcp_fuzzer/fuzz_engine/strategy/aggressive/tool_strategy.py(1 hunks)mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py(1 hunks)mcp_fuzzer/fuzz_engine/strategy/schema_parser.py(1 hunks)tests/test_invariants.py(1 hunks)tests/test_realistic_strategies.py(2 hunks)tests/test_schema_parser.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py (1)
mcp_fuzzer/fuzz_engine/strategy/schema_parser.py (1)
make_fuzz_strategy_from_jsonschema(32-110)
tests/test_invariants.py (1)
mcp_fuzzer/fuzz_engine/invariants.py (7)
InvariantViolation(41-47)check_response_validity(50-99)check_error_type_correctness(102-155)check_response_schema_conformity(158-182)verify_response_invariants(185-215)verify_batch_responses(218-244)check_state_consistency(247-281)
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (1)
mcp_fuzzer/fuzz_engine/invariants.py (2)
verify_response_invariants(185-215)InvariantViolation(41-47)
tests/test_schema_parser.py (1)
mcp_fuzzer/fuzz_engine/strategy/schema_parser.py (8)
make_fuzz_strategy_from_jsonschema(32-110)_handle_enum(113-135)_handle_string_type(253-301)_handle_integer_type(439-499)_handle_number_type(502-567)_handle_boolean_type(570-590)_handle_array_type(186-250)_handle_object_type(138-183)
mcp_fuzzer/fuzz_engine/strategy/aggressive/tool_strategy.py (1)
mcp_fuzzer/fuzz_engine/strategy/schema_parser.py (1)
make_fuzz_strategy_from_jsonschema(32-110)
tests/test_realistic_strategies.py (1)
mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py (4)
base64_strings(19-42)fuzz_tool_arguments_realistic(140-237)timestamp_strings(78-102)uuid_strings(45-75)
🪛 LanguageTool
docs/architecture.md
[grammar] ~247-~247: There might be a mistake here.
Context: ...d invariants and checks for fuzz testing - executor.py: Provides asynchronous execution framew...
(QB_NEW_EN)
[grammar] ~279-~279: There might be a mistake here.
Context: ...r, integer, boolean, array, object, null - String constraints: minLength, maxLength...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...s: minLength, maxLength, pattern, format - Number/Integer constraints: minimum, max...
(QB_NEW_EN)
[grammar] ~281-~281: There might be a mistake here.
Context: ...iveMinimum, exclusiveMaximum, multipleOf - Array constraints: minItems, maxItems, u...
(QB_NEW_EN)
[grammar] ~282-~282: There might be a mistake here.
Context: ...traints: minItems, maxItems, uniqueItems - Object constraints: required properties,...
(QB_NEW_EN)
[grammar] ~283-~283: There might be a mistake here.
Context: ...properties, minProperties, maxProperties - Schema combinations: oneOf, anyOf, allOf...
(QB_NEW_EN)
[grammar] ~284-~284: There might be a mistake here.
Context: ...Schema combinations: oneOf, anyOf, allOf - Enums and constants The module supports...
(QB_NEW_EN)
[grammar] ~295-~295: There might be a mistake here.
Context: ...ponses follow JSON-RPC 2.0 specification - check_error_type_correctness: Verifies error responses have correct ...
(QB_NEW_EN)
[grammar] ~296-~296: There might be a mistake here.
Context: ...sponses have correct structure and codes - check_response_schema_conformity: Validates responses against JSON schem...
(QB_NEW_EN)
[grammar] ~297-~297: There might be a mistake here.
Context: ...esponses against JSON schema definitions - verify_response_invariants: Orchestrates multiple invariant checks...
(QB_NEW_EN)
[grammar] ~298-~298: There might be a mistake here.
Context: ...le invariant checks on a single response - verify_batch_responses: Applies invariant checks to batches of...
(QB_NEW_EN)
[grammar] ~299-~299: There might be a mistake here.
Context: ...invariant checks to batches of responses - check_state_consistency: Ensures server state remains consisten...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (63)
docs/architecture.md (3)
245-249: Docs: New invariants module reference looks good.Reference to fuzz_engine/invariants.py fits the “Core Components” section. No action needed.
256-258: Flow step rename is clear; ensure consistency across docs/UI.You renamed step 5 to “Invariant Verification” and shifted “Analysis” to step 6. Verify other docs/CLI output don’t still reference the old step numbers.
268-269: Path check for schema_parser reference.Confirm mcp_fuzzer/fuzz_engine/strategy/schema_parser.py exists and is included in package builds (setup.cfg/pyproject).
mcp_fuzzer/fuzz_engine/invariants.py (1)
24-36: Optional jsonschema dependency handling is fine.Graceful fallback with HAVE_JSONSCHEMA + placeholder keeps callers simple.
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (1)
17-17: Importing invariants utilities is correct.No concerns.
tests/test_invariants.py (28)
1-18: LGTM! Well-structured test imports.The file header and imports are properly organized, importing all necessary invariants module components for comprehensive testing.
23-26: LGTM! Valid JSON-RPC 2.0 result response test.The test correctly validates a well-formed JSON-RPC response with all required fields.
28-35: LGTM! Valid JSON-RPC 2.0 error response test.The test properly validates an error response with the correct structure including code and message fields.
37-40: LGTM! Valid JSON-RPC notification test.The test correctly validates a notification (response without id, result, or error fields).
42-45: LGTM! Proper None response validation.The test correctly verifies that None responses raise an InvariantViolation.
47-51: LGTM! Invalid JSON-RPC version test.The test properly validates that non-2.0 JSON-RPC versions raise an InvariantViolation.
53-60: LGTM! Missing ID in error response test.The test correctly validates that error responses must include an ID field per JSON-RPC 2.0 specification.
62-71: LGTM! Mutual exclusion of result and error test.The test properly validates that a response cannot contain both result and error fields.
73-77: LGTM! Missing result and error test.The test correctly validates that non-notification responses must have either a result or error field.
79-83: LGTM! Valid error structure test.The test properly validates a well-formed error object.
84-86: LGTM! None error handling test.The test correctly validates that None errors pass validation (no error is valid).
88-92: LGTM! Missing error code validation.The test properly validates that error objects must contain a code field.
94-98: LGTM! Missing error message validation.The test correctly validates that error objects must contain a message field.
100-104: LGTM! Invalid error code type test.The test properly validates that error codes must be integers.
106-110: LGTM! Invalid error message type test.The test correctly validates that error messages must be strings.
112-117: LGTM! Unexpected error code test.The test properly validates that error codes must match expected values when specified.
119-136: LGTM! Valid schema conformity test.The test correctly validates schema conformity with proper mocking of the jsonschema_validate function.
138-156: LGTM! Invalid schema conformity test.The test properly validates that schema validation errors raise InvariantViolation exceptions.
158-173: LGTM! Graceful handling of missing jsonschema package.The test correctly validates that the system logs a warning and returns True when jsonschema is not installed.
175-192: LGTM! Comprehensive invariant verification test.The test properly validates that all invariant checks are called in the correct order.
194-216: LGTM! Error response invariant verification test.The test correctly validates that error type correctness is checked for error responses.
218-233: LGTM! Batch response validation test.The test properly validates that all responses in a batch are processed correctly.
235-251: LGTM! Mixed batch response validation test.The test correctly handles batches with both valid and invalid responses, properly capturing error messages.
253-257: LGTM! Valid state consistency test.The test properly validates that consistent states pass validation, including when new keys are added.
259-266: LGTM! State consistency with allowed changes test.The test correctly validates that explicitly allowed changes pass validation.
268-273: LGTM! Missing key in state test.The test properly validates that missing keys in the after_state raise an InvariantViolation.
275-280: LGTM! Unexpected state change test.The test correctly validates that unexpected changes to non-allowed keys raise an InvariantViolation.
283-285: LGTM! Standard test runner.The test file properly includes the standard unittest main entry point.
tests/test_schema_parser.py (20)
1-20: LGTM! Well-organized imports.The imports are properly structured, importing all necessary components from the schema_parser module for comprehensive testing.
25-52: LGTM! Comprehensive basic type tests.The test thoroughly validates all basic JSON Schema types including string, integer, number, boolean, and null.
54-61: LGTM! Array generation test.The test properly validates array generation with type checking for array items.
63-86: LGTM! Object generation with required properties test.The test comprehensively validates object generation including required properties and type checking.
88-95: LGTM! Enum value selection test.The test correctly validates that enum values are selected from the provided options in realistic mode.
97-123: LGTM! Schema combination tests.The test properly validates oneOf, anyOf, and allOf schema combinations.
125-142: LGTM! String format tests.The test correctly validates email and URI format generation with appropriate format-specific checks.
144-153: LGTM! String constraint validation.The test properly validates that minLength and maxLength constraints are respected.
155-164: LGTM! Number constraint validation.The test correctly validates that minimum and maximum constraints are respected for integers.
166-180: LGTM! Array constraint validation.The test properly validates that minItems and maxItems constraints are respected.
204-230: LGTM! Phase comparison test.The test effectively validates different behaviors between realistic and aggressive phases.
232-236: LGTM! Enum handler test.The test correctly validates the _handle_enum function.
238-244: LGTM! String type handler test.The test properly validates that _handle_string_type respects constraints.
246-252: LGTM! Integer type handler test.The test correctly validates that _handle_integer_type respects constraints.
254-260: LGTM! Number type handler test.The test properly validates that _handle_number_type respects constraints.
262-265: LGTM! Boolean type handler test.The test correctly validates boolean generation.
267-278: LGTM! Array type handler test.The test properly validates that _handle_array_type respects constraints.
280-289: LGTM! Object type handler test.The test correctly validates that _handle_object_type generates required properties.
291-399: LGTM! Comprehensive nested schema test.This is an excellent, thorough test that validates complex nested schemas with multiple levels of nesting, various constraints, and different property types. The test properly checks each level of the generated structure.
402-404: LGTM! Standard test runner.The file properly includes the standard unittest main entry point.
docs/reference.md (4)
116-117: LGTM! New module documentation added.The schema_parser.py module is properly documented in the package layout.
124-124: LGTM! Invariants module documented.The invariants.py module is properly documented in the package layout.
148-185: LGTM! Comprehensive Schema Parser documentation.The documentation thoroughly covers the schema parser features with clear examples and usage patterns.
186-215: LGTM! Well-documented Invariants System.The invariants system documentation is comprehensive with clear examples demonstrating proper usage.
mcp_fuzzer/fuzz_engine/strategy/aggressive/tool_strategy.py (4)
245-246: LGTM! Schema parser integration.The import is properly placed within the function to avoid circular dependencies.
249-250: LGTM! Schema-driven fuzzing approach.Good integration with the schema parser using the aggressive phase for malicious data generation.
252-254: LGTM! Robust type checking.Good defensive programming to ensure the result is always a dictionary.
287-301: LGTM! Malicious field injection.Good security testing approach by injecting prototype pollution and other attack vectors.
mcp_fuzzer/fuzz_engine/strategy/schema_parser.py (2)
169-183: Aggressive extra props risk violating additionalProperties=falseIf schemas set additionalProperties=false, adding proto/constructor/eval breaks validity in aggressive mode (maybe intentional). Confirm desired behavior.
If unintended, gate on schema.get("additionalProperties", True) before injecting extras.
111-121: Edge-case payloads include XSS/SQL strings; confirm test/CI sanitizationAggressive strings include script and injection payloads. Ensure any consumer logs/HTML rendering escapes them.
| def check_response_validity(response: Any) -> bool: | ||
| """ | ||
| Check if a response is valid according to expected formats. | ||
| Args: | ||
| response: The response to check | ||
| Returns: | ||
| bool: True if the response is valid, False otherwise | ||
| Raises: | ||
| InvariantViolation: If the response is invalid | ||
| """ | ||
| # Check if response is None | ||
| if response is None: | ||
| raise InvariantViolation("Response is None") | ||
|
|
||
| # For JSON-RPC responses, check if they have the required fields | ||
| if isinstance(response, dict): | ||
| if "jsonrpc" in response: | ||
| # This appears to be a JSON-RPC response | ||
| if response.get("jsonrpc") != "2.0": | ||
| raise InvariantViolation( | ||
| f"Invalid JSON-RPC version: {response.get('jsonrpc')}", response | ||
| ) | ||
|
|
||
| # Check for id if it's not a notification | ||
| if "id" not in response and "error" in response: | ||
| raise InvariantViolation( | ||
| "JSON-RPC error response missing 'id' field", response | ||
| ) | ||
|
|
||
| # Check that response has either result or error, but not both | ||
| has_result = "result" in response | ||
| has_error = "error" in response | ||
|
|
||
| if has_result and has_error: | ||
| raise InvariantViolation( | ||
| "JSON-RPC response cannot have both 'result' and 'error'", response | ||
| ) | ||
|
|
||
| if not has_result and not has_error: | ||
| # Notifications don't require result or error | ||
| if "id" in response: | ||
| raise InvariantViolation( | ||
| "JSON-RPC response must have either 'result' or 'error'", | ||
| response, | ||
| ) | ||
|
|
||
| return True | ||
|
|
There was a problem hiding this comment.
JSON-RPC response validation misses required id and allows impossible “notification response”.
- Accepts dicts without result/error and no id (responses to notifications must not exist).
- Doesn’t require id for successful result.
- Doesn’t validate id type (int | str | null).
def check_response_validity(response: Any) -> bool:
@@
- # For JSON-RPC responses, check if they have the required fields
+ # For JSON-RPC responses, check required fields
if isinstance(response, dict):
if "jsonrpc" in response:
# This appears to be a JSON-RPC response
if response.get("jsonrpc") != "2.0":
raise InvariantViolation(
f"Invalid JSON-RPC version: {response.get('jsonrpc')}", response
)
- # Check for id if it's not a notification
- if "id" not in response and "error" in response:
- raise InvariantViolation(
- "JSON-RPC error response missing 'id' field", response
- )
-
- # Check that response has either result or error, but not both
+ # Must have either result or error, but not both
has_result = "result" in response
has_error = "error" in response
if has_result and has_error:
raise InvariantViolation(
"JSON-RPC response cannot have both 'result' and 'error'", response
)
- if not has_result and not has_error:
- # Notifications don't require result or error
- if "id" in response:
- raise InvariantViolation(
- "JSON-RPC response must have either 'result' or 'error'",
- response,
- )
+ if not has_result and not has_error:
+ # A response must have result or error; notifications have no response
+ raise InvariantViolation(
+ "JSON-RPC response must contain 'result' or 'error'", response
+ )
+
+ # id is required for any response (result or error)
+ if "id" not in response:
+ raise InvariantViolation("JSON-RPC response missing 'id'", response)
+ if not isinstance(response["id"], (int, str)) and response["id"] is not None:
+ raise InvariantViolation(
+ f"JSON-RPC id must be int, str, or null; got {type(response['id'])}",
+ response,
+ )
+ else:
+ # If dict but not a JSON-RPC envelope, optionally treat as invalid
+ raise InvariantViolation("Unexpected non JSON-RPC response object", response)
- return True
+ else:
+ raise InvariantViolation(f"Unexpected response type: {type(response)}", response)
+ return TrueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In mcp_fuzzer/fuzz_engine/invariants.py around lines 50 to 100, the JSON-RPC
validation currently permits impossible "notification responses", doesn't
require an id for successful results, and doesn't validate the id's type; update
the logic so that any response dict containing "result" or "error" MUST include
an "id" (i.e., responses to requests require id), and any dict lacking both
"result" and "error" should be treated as invalid (notifications must not have a
response), and additionally validate that when "id" is present its type is int,
str or None (null) only; implement these checks by enforcing: if has_result or
has_error then require "id" and validate its type, if neither present raise
InvariantViolation, and if "id" absent then ensure there is no "result" or
"error" (treat as a notification and reject any associated fields).
| from ..schema_parser import make_fuzz_strategy_from_jsonschema | ||
|
|
||
| schema = tool.get("inputSchema", {}) | ||
| properties = schema.get("properties", {}) | ||
| required = schema.get("required", []) | ||
|
|
||
| args = {} | ||
| # Use the enhanced schema parser to generate realistic values | ||
| args = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | ||
|
|
||
| for prop_name, prop_spec in properties.items(): | ||
| prop_type = prop_spec.get("type", "string") | ||
| # If the schema parser returned something other than a dict, create a default dict | ||
| if not isinstance(args, dict): | ||
| args = {} |
There was a problem hiding this comment.
Guard against non-dict or None schemas and parser failures
tool.get("inputSchema") can be None or non-dict, causing errors in the parser or subsequent .get() calls. Add coercion and a safe fallback.
- from ..schema_parser import make_fuzz_strategy_from_jsonschema
+ from ..schema_parser import make_fuzz_strategy_from_jsonschema
@@
- schema = tool.get("inputSchema", {})
+ schema = tool.get("inputSchema")
+ if not isinstance(schema, dict):
+ schema = {}
@@
- args = make_fuzz_strategy_from_jsonschema(schema, phase="realistic")
+ try:
+ args = make_fuzz_strategy_from_jsonschema(schema, phase="realistic")
+ except Exception:
+ args = {}📝 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 ..schema_parser import make_fuzz_strategy_from_jsonschema | |
| schema = tool.get("inputSchema", {}) | |
| properties = schema.get("properties", {}) | |
| required = schema.get("required", []) | |
| args = {} | |
| # Use the enhanced schema parser to generate realistic values | |
| args = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | |
| for prop_name, prop_spec in properties.items(): | |
| prop_type = prop_spec.get("type", "string") | |
| # If the schema parser returned something other than a dict, create a default dict | |
| if not isinstance(args, dict): | |
| args = {} | |
| from ..schema_parser import make_fuzz_strategy_from_jsonschema | |
| schema = tool.get("inputSchema") | |
| if not isinstance(schema, dict): | |
| schema = {} | |
| # Use the enhanced schema parser to generate realistic values | |
| try: | |
| args = make_fuzz_strategy_from_jsonschema(schema, phase="realistic") | |
| except Exception: | |
| args = {} | |
| # If the schema parser returned something other than a dict, create a default dict | |
| if not isinstance(args, dict): | |
| args = {} |
🤖 Prompt for AI Agents
In mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py around lines
142-151, guard against tool.get("inputSchema") being None or a non-dict and
handle parser failures: coerce schema = tool.get("inputSchema") to a dict by
replacing None or non-dict values with {} before calling
make_fuzz_strategy_from_jsonschema, wrap the parser call in a try/except to
catch exceptions and set args = {} on failure, and finally ensure that if the
parser returns a non-dict you replace it with {} so subsequent args.get() calls
are safe.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
mcp_fuzzer/fuzz_engine/invariants.py (1)
85-94: Fix: disallow impossible “notification responses”.A JSON-RPC response must contain result or error; notifications have no response at all. Allowing a dict with neither is incorrect.
Apply this diff:
- if not has_result and not has_error: - # Check if this is a notification (no id) or a response - if "id" in response: - # Responses must have result or error - raise InvariantViolation( - "JSON-RPC response must have either 'result' or 'error'", - response, - ) + if not has_result and not has_error: + raise InvariantViolation( + "JSON-RPC response must contain 'result' or 'error'", response + )
🧹 Nitpick comments (6)
mcp_fuzzer/fuzz_engine/invariants.py (6)
26-29: Improve schema error reporting: include failing path.Catch jsonschema.ValidationError to surface the exact path and concise message.
Apply this diff:
- from jsonschema import validate as jsonschema_validate - - HAVE_JSONSCHEMA = True + from jsonschema import validate as jsonschema_validate + from jsonschema.exceptions import ValidationError + HAVE_JSONSCHEMA = True- except Exception as e: - raise InvariantViolation( - f"Response does not conform to schema: {e}", response - ) + except ValidationError as e: + path = ".".join(str(p) for p in e.path) + raise InvariantViolation( + f"Schema violation at {path or '<root>'}: {e.message}", response + ) + except Exception as e: + raise InvariantViolation( + f"Response does not conform to schema: {e}", response + )Also applies to: 196-199
32-36: Remove unused placeholder.The local jsonschema_validate stub is never called; simplify by dropping it.
Apply this diff:
- # Define a placeholder function when jsonschema is not available - def jsonschema_validate(instance, schema): - """Placeholder when jsonschema is not available.""" - pass
231-233: Validate schema against result payload; skip when error present.Typical usage validates the successful result rather than the whole envelope.
Apply this diff:
- if schema is not None: - check_response_schema_conformity(response, schema) + if schema is not None and not (isinstance(response, dict) and "error" in response): + target = ( + response["result"] + if isinstance(response, dict) and "result" in response + else response + ) + check_response_schema_conformity(target, schema)
1-1: Shebang not needed in a library module.Drop the shebang to avoid implying executability.
Apply this diff:
-#!/usr/bin/env python3
286-289: Optionally allow expected removals.Permit key removals when explicitly listed in expected_changes.
Apply this diff:
- for key in before_state: - if key not in after_state: - raise InvariantViolation(f"Key '{key}' missing in after_state") + for key in before_state: + if key not in after_state: + if expected_changes and key in expected_changes: + continue + raise InvariantViolation(f"Key '{key}' missing in after_state")
98-106: Exclude bool IDs; accept int, float, str, or null per JSON-RPC 2.0. Boolean is a subclass of int—exclude it explicitly—then checkisinstance(id_value, (int, float, str)); JSON-RPC 2.0 allows Number (including fractional) but “Numbers SHOULD NOT contain fractional parts” so you can optionally reject floats with non-zero fractional component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py(3 hunks)mcp_fuzzer/fuzz_engine/invariants.py(1 hunks)mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py(1 hunks)mcp_fuzzer/fuzz_engine/strategy/schema_parser.py(1 hunks)tests/test_invariants.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- mcp_fuzzer/fuzz_engine/strategy/schema_parser.py
- tests/test_invariants.py
- mcp_fuzzer/fuzz_engine/strategy/realistic/tool_strategy.py
- mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (1)
mcp_fuzzer/fuzz_engine/invariants.py (1)
205-235: Solid composition of invariants.Combining structural, error, and (optional) schema checks into a single entry point is clean and testable. Nice.
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (1)
311-312: Alignserver_responsetype hints with list responses
Update_send_fuzzed_request,_create_fuzz_resultand theFuzzDataResult.server_responsefield to useOptional[Union[Dict[str, Any], List[Any]]]instead ofOptional[Dict[str, Any]].
♻️ Duplicate comments (2)
mcp_fuzzer/fuzz_engine/invariants.py (2)
85-116: Treat any JSON-RPC message with 'method' as invalid in a response context; require id (incl. float) and reject bool ids.Current logic accepts “notification responses” and requests; also disallows float ids and allows bool via int subclassing.
Apply:
- # Determine if this is a notification, request, or response - has_id = "id" in response - has_method = "method" in response - - # If it has method but no id, it's a notification (valid) - if has_method and not has_id: - # Notifications don't need result/error - return True - - # If it has id but no method, it's a response and needs result or error - if not has_result and not has_error and has_id and not has_method: - # Responses must have result or error - raise InvariantViolation( - "JSON-RPC response must have either 'result' or 'error'", - response, - ) - - # id is required for any response (result or error) - # but not for notifications (which have method but no id) - if (has_result or has_error) and not has_method: - if "id" not in response: - raise InvariantViolation("JSON-RPC response missing 'id'", response) - if ( - not isinstance(response["id"], (int, str)) - and response["id"] is not None - ): - raise InvariantViolation( - f"JSON-RPC id must be int, str, or null; " - f"got {type(response['id'])}", - response, - ) + # Responses must NOT include 'method' (requests/notifications) + has_id = "id" in response + has_method = "method" in response + if has_method: + raise InvariantViolation( + "Unexpected JSON-RPC request/notification object in response context", + response, + ) + + # Response must include either result or error + if not has_result and not has_error: + raise InvariantViolation( + "JSON-RPC response must contain 'result' or 'error'", + response, + ) + + # id is required for any response (result or error) + if not has_id: + raise InvariantViolation("JSON-RPC response missing 'id'", response) + + rid = response["id"] + if rid is not None and not ( + (isinstance(rid, (int, float)) and not isinstance(rid, bool)) + or isinstance(rid, str) + ): + raise InvariantViolation( + f"JSON-RPC id must be int, float, str, or null; got {type(rid)}", + response, + )
157-162: Reject bool error codes; bool is a subclass of int.- if not isinstance(error["code"], int): - raise InvariantViolation( - (f"JSON-RPC error code must be an integer, got {type(error['code'])}"), - error, - ) + code = error["code"] + if isinstance(code, bool) or not isinstance(code, int): + raise InvariantViolation( + f"JSON-RPC error code must be an integer (bool not allowed); got {type(code)}", + error, + )
🧹 Nitpick comments (2)
mcp_fuzzer/fuzz_engine/invariants.py (1)
306-327: Optional: simplify state checks with set operations for readability.Alternative implementation (outside-diff):
before_keys = set(before_state) after_keys = set(after_state) missing = before_keys - after_keys if missing: raise InvariantViolation(f"Key(s) missing in after_state: {sorted(missing)}") added = after_keys - before_keys if added and (not expected_changes or not set(expected_changes) & added): raise InvariantViolation(f"Unexpected key(s) added to after_state: {sorted(added)}") changed = {k for k in (before_keys & after_keys) if before_state[k] != after_state[k]} if changed and (not expected_changes or not changed <= set(expected_changes)): unexpected = changed - set(expected_changes or []) raise InvariantViolation(f"Unexpected change(s): {sorted(unexpected)}")mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (1)
220-252: Pass schema and expected codes to validators; include batch indices; avoid extra create_task.Improves signal and aligns with schema/invariants objectives; retains timeout.
- # Verify invariants if we have a server response + # Verify invariants if we have a server response invariant_violations = [] if server_response is not None and not generate_only: + expected_codes = None + response_schema = None + if isinstance(fuzz_data, dict): + expected_codes = fuzz_data.get("expected_error_codes") + response_schema = fuzz_data.get("response_schema") try: if isinstance(server_response, list): - try: - # Handle batch responses with timeout to prevent hanging - batch = await asyncio.wait_for( - asyncio.create_task( - verify_batch_responses(server_response) - ), - timeout=5.0, # 5 second timeout for batch validation - ) - viols = [str(v) for k, v in batch.items() if v is not True] + try: + # Handle batch responses with timeout to prevent hanging + batch = await asyncio.wait_for( + verify_batch_responses( + server_response, expected_codes, response_schema + ), + timeout=5.0, # 5 second timeout for batch validation + ) + viols = [ + f"[batch:{k}] {v}" for k, v in batch.items() if v is not True + ] invariant_violations.extend(viols) except asyncio.TimeoutError: invariant_violations.append("Batch validation timed out") self._logger.warning( "Batch validation timeout in %s run %s", protocol_type, run_index + 1, ) else: - verify_response_invariants(server_response) + verify_response_invariants( + server_response, expected_codes, response_schema + )Optional: define a module-level constant and reuse:
BATCH_VALIDATION_TIMEOUT = 5.0and use it in wait_for.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py(3 hunks)mcp_fuzzer/fuzz_engine/invariants.py(1 hunks)tests/test_invariants.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_invariants.py
🧰 Additional context used
🧬 Code graph analysis (1)
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (1)
mcp_fuzzer/fuzz_engine/invariants.py (3)
verify_response_invariants(214-244)InvariantViolation(41-47)verify_batch_responses(247-284)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (3)
mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py (3)
17-21: LGTM: invariants imports added.
258-261: LGTM: attach invariant violations to result only when present.
371-371: LGTM: default invariant_violations field in result.
mcp_fuzzer/fuzz_engine/invariants.py
Outdated
| # Check if error is None | ||
| if error is None: | ||
| return True | ||
|
|
There was a problem hiding this comment.
Disallow null error objects.
If “error” key exists, value must be an object per JSON-RPC 2.0. Null currently passes.
- # Check if error is None
- if error is None:
- return True
+ # 'error' must be an object when present per JSON-RPC 2.0
+ if error is None:
+ raise InvariantViolation("JSON-RPC error must be an object; got null", error)📝 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.
| # Check if error is None | |
| if error is None: | |
| return True | |
| # 'error' must be an object when present per JSON-RPC 2.0 | |
| if error is None: | |
| raise InvariantViolation("JSON-RPC error must be an object; got null", error) |
🤖 Prompt for AI Agents
In mcp_fuzzer/fuzz_engine/invariants.py around lines 145 to 148, the current
check returns True when error is None which allows a null error value; change
the logic so a present "error" key with value None is rejected (return False)
and instead require the error value to be a mapping/object (e.g., use
isinstance(error, dict)); update the check to return False for None and only
accept dict-like error objects (optionally also validate required fields like
"code" and "message" if desired).
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
mcp_fuzzer/fuzz_engine/invariants.py (1)
76-116: Treat only true JSON-RPC responses as valid; reject notifications/requests and bool ids.Current logic returns True for notifications and silently accepts request envelopes; also allows boolean ids. For a “response” validator, messages with 'method' should be rejected, and ids must not be bools.
Apply this diff:
@@ - # Determine if this is a notification, request, or response - has_id = "id" in response - has_method = "method" in response - - # If it has method but no id, it's a notification (valid) - if has_method and not has_id: - # Notifications don't need result/error - return True + # Determine if this is a notification, request, or response + has_id = "id" in response + has_method = "method" in response + + # A response must not contain 'method'; reject requests/notifications here + if has_method: + raise InvariantViolation( + "Unexpected JSON-RPC request/notification object; expected a response", + response, + ) @@ - # If it has id but no method, it's a response and needs result or error - if not has_result and not has_error and has_id and not has_method: - # Responses must have result or error - raise InvariantViolation( - "JSON-RPC response must have either 'result' or 'error'", - response, - ) + # A response must include either 'result' or 'error' (exclusively) + if not has_result and not has_error: + raise InvariantViolation( + "JSON-RPC response must contain 'result' or 'error'", + response, + ) @@ - # id is required for any response (result or error) - # but not for notifications (which have method but no id) - if (has_result or has_error) and not has_method: - if "id" not in response: - raise InvariantViolation("JSON-RPC response missing 'id'", response) - if ( - not isinstance(response["id"], (int, str)) - and response["id"] is not None - ): - raise InvariantViolation( - f"JSON-RPC id must be int, str, or null; " - f"got {type(response['id'])}", - response, - ) + # 'id' is required for any response (result or error) + if (has_result or has_error): + if "id" not in response: + raise InvariantViolation("JSON-RPC response missing 'id'", response) + id_val = response["id"] + if id_val is not None and (isinstance(id_val, bool) or not isinstance(id_val, (int, str))): + raise InvariantViolation( + f"JSON-RPC id must be int, str, or null (bool not allowed); got {type(id_val)}", + response, + )
🧹 Nitpick comments (3)
mcp_fuzzer/fuzz_engine/invariants.py (3)
202-213: Optional: Fail fast when a schema is provided but jsonschema is unavailable.Currently logs a warning and treats as valid, which can mask contract violations. Consider an opt-in strict mode.
Example change:
-def check_response_schema_conformity(response: Any, schema: Dict[str, Any]) -> bool: +def check_response_schema_conformity(response: Any, schema: Dict[str, Any], *, require_validator: bool = False) -> bool: @@ - else: - logger.warning("jsonschema package not installed, skipping schema validation") - return True + else: + if require_validator: + raise InvariantViolation("jsonschema not installed; cannot validate against provided schema", response) + logger.warning("jsonschema package not installed, skipping schema validation") + return TrueIf you like, I can propagate require_validator through verify_response_invariants/verify_batch_responses.
288-306: Differentiate added keys from mutated keys.expected_changes currently governs both value changes and additions, which is ambiguous.
Apply this diff:
-def check_state_consistency( - before_state: Dict[str, Any], - after_state: Dict[str, Any], - expected_changes: Optional[List[str]] = None, -) -> bool: +def check_state_consistency( + before_state: Dict[str, Any], + after_state: Dict[str, Any], + expected_changes: Optional[List[str]] = None, + allowed_added_keys: Optional[List[str]] = None, +) -> bool: @@ - # Check if any unexpected keys were added + # Check if any unexpected keys were added for key in after_state: if key not in before_state: - if not expected_changes or key not in expected_changes: + if not allowed_added_keys or key not in allowed_added_keys: raise InvariantViolation(f"Unexpected key '{key}' added to after_state")Update the docstring to describe allowed_added_keys; keep expected_changes semantics for mutated values.
Also applies to: 312-317
57-62: Docstrings say “False otherwise” but functions raise instead.Align return docs to “True on success; raises InvariantViolation otherwise.”
Example for one function:
- Returns: - bool: True if the response is valid, False otherwise + Returns: + bool: True on success @@ - InvariantViolation: If the response is invalid + InvariantViolation: If validation failsApply similarly to other functions noted.
Also applies to: 139-144, 196-201, 228-233, 261-264, 301-306
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
mcp_fuzzer/fuzz_engine/invariants.py(1 hunks)tests/test_invariants.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_invariants.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (1)
mcp_fuzzer/fuzz_engine/invariants.py (1)
145-185: JSON-RPC error validation looks solid.Null error objects are rejected; non-dict errors raise; bool codes are rejected; required fields validated. Good alignment with spec.
| async def verify_batch_responses( | ||
| responses: List[Any], | ||
| expected_error_codes: Optional[List[int]] = None, | ||
| schema: Optional[Dict[str, Any]] = None, | ||
| ) -> Dict[int, Union[bool, str]]: | ||
| """ | ||
| Verify invariants for a batch of responses asynchronously. | ||
| Args: | ||
| responses: The responses to verify | ||
| expected_error_codes: Optional list of expected error codes | ||
| schema: Optional schema to validate against | ||
| Returns: | ||
| Dict[int, Union[bool, str]]: A dictionary mapping response indices to | ||
| verification results (True if valid, error message if invalid) | ||
| """ | ||
| results = {} | ||
|
|
||
| # Guard against None or empty responses | ||
| if not responses: | ||
| return results | ||
|
|
||
| # Guard against non-list responses | ||
| if not isinstance(responses, list): | ||
| results[0] = f"Expected a list of responses, got {type(responses)}" | ||
| return results | ||
|
|
||
| for i, response in enumerate(responses): | ||
| try: | ||
| verify_response_invariants(response, expected_error_codes, schema) | ||
| results[i] = True | ||
| except InvariantViolation as e: | ||
| results[i] = str(e) | ||
| except Exception as e: | ||
| results[i] = f"Unexpected error: {str(e)}" | ||
|
|
||
| return results | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional: Make the async batch truly concurrent or make it sync.
The coroutine does no awaits. Either drop async or parallelize with asyncio.to_thread.
Example (requires import asyncio):
-async def verify_batch_responses(
+async def verify_batch_responses(
@@
- for i, response in enumerate(responses):
- try:
- verify_response_invariants(response, expected_error_codes, schema)
- results[i] = True
- except InvariantViolation as e:
- results[i] = str(e)
- except Exception as e:
- results[i] = f"Unexpected error: {str(e)}"
+ async def _wrap(resp):
+ try:
+ return await asyncio.to_thread(verify_response_invariants, resp, expected_error_codes, schema)
+ except InvariantViolation as e:
+ return e
+ except Exception as e:
+ return e
+
+ tasks = [ _wrap(r) for r in responses ]
+ for i, res in enumerate(await asyncio.gather(*tasks, return_exceptions=True)):
+ if res is True:
+ results[i] = True
+ elif isinstance(res, Exception):
+ results[i] = f"{res}"
+ else:
+ results[i] = True🏁 Script executed:
#!/bin/bash
# Ensure callers await verify_batch_responses
rg -nP -C1 '\bverify_batch_responses\s*\('Length of output: 952
Await and handle verify_batch_responses properly
- In mcp_fuzzer/fuzz_engine/fuzzer/protocol_fuzzer.py:229, you call
asyncio.create_task(verify_batch_responses(server_response))
without awaiting or capturing the task—any invariant failures are dropped. Await the coroutine or gather its result so errors propagate or results can be inspected. verify_batch_responsesis declaredasyncbut has noawaitinside and thus blocks the event loop. Either make it a normal function or introduce true async work (e.g., wrapverify_response_invariantsinasyncio.to_threadand useasyncio.gather).
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. |
Implement comprehensive JSON Schema parser with support for types, constraints, and combinations
Add property-based invariants for response validation and error checking
Update documentation with schema parser and invariants details
Fix tests and linting issues
Fixes Add property-based invariants and checks #10: Add property-based invariants and checks
Fixes Enhance JSON Schema parsing for fuzz strategies #12: Enhance JSON Schema parsing for fuzz strategies
Fixes Type Safety and Code Quality Enhancements #53: Type safety and code quality enhancements
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests