Skip to content

add McpProtection plugin#2977

Merged
predic8 merged 71 commits into
masterfrom
mcp-protection
Jul 1, 2026
Merged

add McpProtection plugin#2977
predic8 merged 71 commits into
masterfrom
mcp-protection

Conversation

@christiangoerdes

@christiangoerdes christiangoerdes commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added JSON-RPC protection with allow/deny rules, batch size enforcement, and configurable request/response/error JSON Schema validation.
    • Introduced new JSON-RPC method and schema configuration components.
    • Added MCP protection for JSON-RPC, including tools/list filtering of unauthorized entries.
  • Bug Fixes
    • Improved JSON schema generation and YAML/JSONPath error locations for excluded/invisible elements and complex keys.
    • Enhanced “other attributes” map handling, including correct additionalProperties schema behavior and safer type coercion.
  • Documentation
    • Updated JSON-RPC security tutorials and added supporting launcher scripts.
  • Tests
    • Expanded JSON-RPC, MCP, and JSONPath/error rendering test coverage.

christiangoerdes and others added 30 commits May 28, 2026 13:14
…PCValidator, and enhance method null check in Rule
…pc-protection

# Conflicts:
#	core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCParams.java
…or` and add helper method `getJsonVisibleChildElementSpecs`.
…proper error handling and add corresponding test.
… parameter validation, and batch size limits
…d update associated logic, tests, and documentation
…`error` with updated tests and supporting classes
…lace with `schemaValidation` supporting `params`, `response`, and `error` validation. Update tests and tutorials accordingly.
Comment thread core/src/main/java/com/predic8/membrane/core/util/config/allowdeny/Rule.java Outdated
…dling for non-`tools/list` responses in `MCPProtectionInterceptor`.
…od with static `getPayloadType`, remove unused methods, and improve code consistency.
…nly/write actions, adjust schemas, refine examples, and update test cases
…en error and success schemas are configured, and add corresponding test case.
… manipulation, improve `findLastSegmentStart` logic, and add unit test.
# Conflicts:
#	distribution/tutorials/README.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MCPProtectionInterceptorTest.java (1)

398-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the response body stays unchanged here.

This only checks the return value. A regression that rewrites non-tools/list JSON but still returns CONTINUE would still pass, so the negative path is not really covered.

Suggested fix
         exc.setResponse(Response.ok()
                 .contentType(APPLICATION_JSON)
                 .body(body)
                 .build());

         assertEquals(CONTINUE, interceptor.handleResponse(exc));
+        assertEquals(body, exc.getResponse().getBodyAsStringDecoded());
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MCPProtectionInterceptorTest.java`
around lines 398 - 423, The negative-path test in MCPProtectionInterceptorTest
only verifies that handleResponse returns CONTINUE, but it does not confirm that
a non-tools/list JSON response remains untouched. Update
leavesNonToolsListResponsesUnchanged to capture the original response body from
Response.ok() and assert after interceptor.handleResponse(exc) that the response
body is still identical, using the existing handleResponse/exc setup to ensure
no rewriting happens for ping responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCProtectionInterceptor.java`:
- Around line 185-190: Invalidate the cached JsonRPCValidator whenever
configuration objects are replaced so validation state stays in sync with the
active config. Update JsonRPCProtectionInterceptor’s setSchemaValidation,
setBatch, and setMethods setters to clear the memoized validator (the field used
by init()/getValidator()) after swapping in new JsonRPCSchemaValidation, batch,
or methods objects, so handleResponse() and request/response validation all use
the same current settings.

---

Nitpick comments:
In
`@core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MCPProtectionInterceptorTest.java`:
- Around line 398-423: The negative-path test in MCPProtectionInterceptorTest
only verifies that handleResponse returns CONTINUE, but it does not confirm that
a non-tools/list JSON response remains untouched. Update
leavesNonToolsListResponsesUnchanged to capture the original response body from
Response.ok() and assert after interceptor.handleResponse(exc) that the response
body is still identical, using the existing handleResponse/exc setup to ensure
no rewriting happens for ping responses.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7a6f94c6-71fd-4bb7-b120-b1233f1fa3f0

📥 Commits

Reviewing files that changed from the base of the PR and between bc77e5f and 271fa07.

📒 Files selected for processing (11)
  • annot/src/main/java/com/predic8/membrane/annot/yaml/error/LineYamlErrorRenderer.java
  • annot/src/test/java/com/predic8/membrane/annot/yaml/error/LineYamlErrorRendererTest.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCProtectionInterceptor.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCSchemaValidation.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCValidator.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MCPProtectionInterceptor.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MCPProtectionValidator.java
  • core/src/test/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCProtectionInterceptorTest.java
  • core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MCPProtectionInterceptorTest.java
  • distribution/src/test/java/com/predic8/membrane/tutorials/security/JsonRpcProtectionTutorialTest.java
  • distribution/tutorials/README.md
💤 Files with no reviewable changes (1)
  • core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCSchemaValidation.java
✅ Files skipped from review due to trivial changes (1)
  • distribution/tutorials/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • distribution/src/test/java/com/predic8/membrane/tutorials/security/JsonRpcProtectionTutorialTest.java
  • annot/src/main/java/com/predic8/membrane/annot/yaml/error/LineYamlErrorRenderer.java
  • core/src/test/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCProtectionInterceptorTest.java
  • core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCValidator.java

christiangoerdes and others added 10 commits June 29, 2026 14:32
…onversion logic, and introduce comprehensive tests for attribute parsing.
…h Base64 URL encoding, handle colliding method names, and add test coverage.
…idator` usage, remove redundant method, and optimize property name matching with static regex pattern.
…Test` and implement `getMapValueType` in `ScalarValueConverter`.
…mentation to include protected endpoint setup instructions.
…date MCP protection functionality with comprehensive test coverage
…n responses, and ensure correct rule matching
christiangoerdes and others added 4 commits June 30, 2026 11:57
# Conflicts:
#	annot/src/main/java/com/predic8/membrane/annot/yaml/ParsingContext.java
#	annot/src/main/java/com/predic8/membrane/annot/yaml/parsing/binding/ScalarValueConverter.java
#	core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCProtectionInterceptor.java
#	core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCSchemaValidation.java
#	core/src/main/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCValidator.java
#	core/src/main/java/com/predic8/membrane/core/util/config/allowdeny/Rule.java
#	core/src/test/java/com/predic8/membrane/core/interceptor/json/rpc/JsonRPCProtectionInterceptorTest.java
@christiangoerdes christiangoerdes requested a review from predic8 June 30, 2026 11:29
@predic8 predic8 merged commit d3b0ea4 into master Jul 1, 2026
5 checks passed
@predic8 predic8 deleted the mcp-protection branch July 1, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants