-
Notifications
You must be signed in to change notification settings - Fork 400
feat: initial revision of adding plugin support. #642
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
Conversation
π MCP Gateway Plugin Framework - PR Review & Testing Guideπ OverviewThis PR implements an MVP plugin framework for MCP Gateway, enabling extensible pre/post processing hooks throughout the request lifecycle. The framework supports both native plugins (Python, run in-process) and external microservice plugins (LLMGuard, OPA, etc). β What's ImplementedCore Framework
Example Plugins
Integration Points
π Not Yet Implemented (Future Work)The following hooks are defined but not implemented:
π§ͺ Testing Instructions1. Setup# Checkout branch
git checkout 319-plugin-framework-experimental
# Install dependencies
make install
# Configure environment
cp .env.example .env
# Ensure PLUGINS_ENABLED=true in .env2. Basic Functionality TestsTest Plugin Framework Disabled: PLUGINS_ENABLED=false python -m mcpgateway
# Expected: Server starts normally, no plugin logsTest Plugin Framework Enabled: PLUGINS_ENABLED=true python -m mcpgateway
# Expected: "Plugin manager initialized with 1 plugins"3. Search/Replace Plugin Test# Create test prompt
curl -X POST http://localhost:8000/prompts \
-H "Content-Type: application/json" \
-d '{
"name": "test_prompt",
"template": "User said: {{ message }}",
"description": "Test prompt for plugin validation"
}'
# Test word replacement
curl -X POST http://localhost:8000/prompts/test_prompt/render \
-H "Content-Type: application/json" \
-d '{
"arguments": {
"message": "This is crap"
}
}'
# Expected: "User said: This is yikes" (crap β crud β yikes)4. Plugin Modes TestEdit
5. API Endpoints Test# List loaded plugins
curl http://localhost:8000/plugins
# Expected: JSON array of plugin detailsβ Merge ChecklistCode Quality
Functionality
Configuration
Integration
Documentation
π Performance Impact# Baseline (no plugins)
PLUGINS_ENABLED=false python -m mcpgateway
time curl -X POST http://localhost:8000/prompts/test_prompt/render -d '{"arguments":{"message":"test"}}'
# With plugins
PLUGINS_ENABLED=true python -m mcpgateway
# Response time should be within 10% of baseline
|
89c0744 to
0809983
Compare
Signed-off-by: Teryl Taylor <[email protected]>
β¦ed some linting issues, updated example plugin with posthook. Signed-off-by: Teryl Taylor <[email protected]>
β¦and type issues. Signed-off-by: Teryl Taylor <[email protected]>
β¦pytest Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Frederico Araujo <[email protected]>
Signed-off-by: Frederico Araujo <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: Teryl Taylor <[email protected]>
Signed-off-by: Frederico Araujo <[email protected]>
3d7af6d to
8b4e818
Compare
Signed-off-by: Frederico Araujo <[email protected]>
π Plugin Framework PR #642 - Review & Testing Guideπ PR Review SummaryThis PR implements an MVP plugin framework for MCP Gateway, delivering the foundation for extensible request processing through pre/post hooks. The implementation focuses on prompt hooks as the initial integration point. β What's Implemented (MVP Scope)Core Framework β
Hook Points (2 of 12 Planned)
Other Future Hook Points and Features
Example Plugins β
Integration & Management
Configuration Features
π΄ Critical Fixes Required Before Merge1. Add Timeout Protection (Prevents hanging)# File: mcpgateway/plugins/framework/manager.py
# In PluginExecutor.execute(), wrap plugin execution:
import asyncio
async def execute(self, plugins: list[PluginRef], payload: T, ...):
for pluginref in plugins:
try:
# ADD THIS: Timeout wrapper
result = await asyncio.wait_for(
plugin_run(pluginref, payload, local_context),
timeout=30 # 30 second timeout
)
except asyncio.TimeoutError:
logger.error(f"Plugin {pluginref.name} timed out after 30s")
if pluginref.plugin.mode == PluginMode.ENFORCE:
violation = PluginViolation(
reason="Plugin timeout",
description=f"Plugin exceeded 30s timeout",
code="PLUGIN_TIMEOUT",
details={}
)
return (PluginResult[T](
continue_processing=False,
violation=violation
), None)
continue # Skip in permissive mode
# ... rest of existing code2. Fix Memory Leak in Context Storage# File: mcpgateway/plugins/framework/manager.py
# Add to PluginManager class:
import time
from typing import Dict, Tuple
class PluginManager:
def __init__(self, config: str = ""):
# ... existing code ...
# ADD THESE:
self._context_store: Dict[str, Tuple[PluginContextTable, float]] = {}
self._last_cleanup = time.time()
async def _cleanup_old_contexts(self):
"""Remove contexts older than 1 hour."""
current_time = time.time()
if current_time - self._last_cleanup < 300: # Cleanup every 5 min
return
expired = [
k for k, (_, ts) in self._context_store.items()
if current_time - ts > 3600
]
for key in expired:
del self._context_store[key]
if expired:
logger.info(f"Cleaned up {len(expired)} expired contexts")
self._last_cleanup = current_time
async def prompt_pre_fetch(self, payload: PromptPrehookPayload, ...):
# ADD THIS at the start:
await self._cleanup_old_contexts()
# ... rest of existing code3. Add Input Validation (Prevent DoS)# File: mcpgateway/plugins/framework/manager.py
# Add at start of prompt_pre_fetch and prompt_post_fetch:
MAX_PAYLOAD_SIZE = 1_000_000 # 1MB limit
async def prompt_pre_fetch(self, payload: PromptPrehookPayload, ...):
# ADD THIS validation:
if payload.args:
total_size = sum(len(str(v)) for v in payload.args.values())
if total_size > MAX_PAYLOAD_SIZE:
raise ValueError(f"Payload size {total_size} exceeds limit")
# ... rest of existing codeπ§ͺ Testing InstructionsSetup# Start server with plugins enabled
export PLUGINS_ENABLED=true
make serve # Starts on port 4444
# Create auth token
export MCPGATEWAY_BEARER_TOKEN=$(python3 -m mcpgateway.utils.create_jwt_token -u admin --secret my-test-key)Test 1: Basic Plugin Functionality# Create test prompt
curl -X POST http://localhost:4444/prompts \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{
"name": "test_prompt",
"template": "User said: {{ user }}",
"argument_schema": {
"type": "object",
"properties": {"user": {"type": "string"}},
"required": ["user"]
}
}'
# Test word replacement (crap β crud β yikes)
curl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"user": "What a crap situation"}'
# Expected: "User said: What a yikes situation"Test 2: Deny List Plugin (Blocking)# Test with blocked word "revolutionary"
curl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"user": "This is revolutionary"}'
# Expected: HTTP 422 with error message about denied wordTest 3: PII Filter Plugin# Test PII masking (if enabled in config)
curl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"user": "My SSN is 123-45-6789 and email is [email protected]"}'
# Expected: SSN and email should be masked (e.g., ***-**-6789, j***[email protected])Test 4: Plugin Modes# Edit plugins/config.yaml and change mode:
# - "enforce" β blocks on violation
# - "permissive" β logs but allows
# - "disabled" β plugin doesn't run
# Restart server after config change
make serve
# Re-run deny list test to see different behaviorsTest 5: Performance Check# Baseline (no plugins)
PLUGINS_ENABLED=false make serve
time curl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-d '{"user": "test"}'
# With plugins
PLUGINS_ENABLED=true make serve
time curl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-d '{"user": "test"}'
# Performance impact should be <10%π§ Merge Process# 1. Checkout PR
gh pr checkout 642
# 2. Update with latest main
git fetch origin main
git rebase origin/main
# Resolve any conflicts if needed
# 3. Run test suite
make autoflake isort black pre-commit
make doctest test smoketest lint-web flake8
make lint-commit
make check-headers
make bandit devskim
make lint target=plugins
make lint TARGET=plugins mcpgateway/plugins
make devpi-install devpi-init devpi-start devpi-setup-user devpi-upload devpi-test
# 4. Apply the 3 critical fixes above
# Edit the files and re-run test suite
# 5. Test fixes work
make serve
# Run curl tests above
# 6. Commit fixes
git add mcpgateway/plugins/framework/manager.py
git commit -m "fix: Add timeout, memory cleanup, and input validation to plugins
- Add 30s timeout for plugin execution
- Implement context cleanup to prevent memory leaks
- Add 1MB payload size validation
- Required safety fixes per PR review"
# 7. Push changes
git push
# 8. Squash and merge
gh pr merge 642 --squash --subject "feat: Add plugin framework for extensible request processing (#642)"β Merge ChecklistCritical (Must Fix):
Testing:
Documentation:
π Post-Merge Feature BacklogPerformance Enhancements
Observability & Monitoring
Safety & Resilience
Configuration & Management
Additional Hook Implementation
External Service Integration
Developer Experience
Admin & Management Features
π― Implementation Status vs Original EpicDelivered in MVP
Not Yet Implemented
Potential issues:Check for race condition in comments below. π Final VerdictAPPROVED FOR MERGE after applying the 3 critical fixes. This MVP successfully delivers the plugin framework foundation with prompt hooks as the initial integration point. The architecture is extensible and well-tested, providing a solid base for future hook implementations and external service integrations. The implementation follows clean architecture principles with proper separation of concerns, comprehensive error handling, and extensive test coverage. The disabled-by-default approach ensures zero impact on existing deployments. I will apply the 3 fixes and push the changes. Once done, we can proceed with the squash and merge. |
Signed-off-by: Mihai Criveti <[email protected]>
Testing regex filteringcurl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"user": "What a crap situation"}'{"messages":[{"role":"user","content":{"type":"text","text":"User said: What a yikes situation"}}],"description":null}Testing blocking:curl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"user": "This is revolutionary"}'{"message":"Prompt execution arguments contains HTML tags that may cause security issues","details":"Pre prompting fetch blocked by plugin DenyListPlugin: deny - Prompt not allowed (A deny word was found in the prompt)"}Testing PII Filteringcurl -X POST http://localhost:4444/prompts/test_prompt \
-H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" \
-H "Content-Type: application/json" \
-d '{"user": "My SSN is 123-45-6789 and email is [email protected]"}'{"messages":[{"role":"user","content":{"type":"text","text":"User said: My SSN is [PII_REDACTED]-**-6789DACTED]-***-***-***-***-6789 and email is j******@example.com"}}],"description":null} |
β¦ework (timeout, memory cleanup, validation) Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
|
There might be a race condition here: |
|
Crated #673 to track next steps. |
crivetimihai
left a comment
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.
Ready to merge.
|
Feature Validation: Tested - Working well. PR Test summary: make serve - pass |
* feat: initial revision of adding plugin support. Signed-off-by: Teryl Taylor <[email protected]> * feat(plugins): added prompt posthook functionality with executor, fixed some linting issues, updated example plugin with posthook. Signed-off-by: Teryl Taylor <[email protected]> * feat(plugins): integrated plugins into prompt service, fixed linting and type issues. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): renamed types.py to plugin_types.py due to conflict in pytest Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): fixed renamed plugin_types module in prompt_service.py Signed-off-by: Teryl Taylor <[email protected]> * feat: add example filter plugin Signed-off-by: Frederico Araujo <[email protected]> * feat: add plugin violation error object Signed-off-by: Frederico Araujo <[email protected]> * feat: added unit tests Signed-off-by: Teryl Taylor <[email protected]> * fix(license): add licensing to unit test files and run lint. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugin): linting issues. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): added yaml dependency for plugins to pyproject.toml Signed-off-by: Teryl Taylor <[email protected]> * test(plugin): added tests for filter plugins Signed-off-by: Teryl Taylor <[email protected]> * test(plugin): add missing config files for plugin tests Signed-off-by: Teryl Taylor <[email protected]> * Add PII filter plugin Signed-off-by: Mihai Criveti <[email protected]> * docs(plugins): updated plugins documentation. Signed-off-by: Teryl Taylor <[email protected]> * fix: include plugin md files in manifest Signed-off-by: Frederico Araujo <[email protected]> * docs: add deny list plugin readme Signed-off-by: Frederico Araujo <[email protected]> * Pre-commit cleanup Signed-off-by: Mihai Criveti <[email protected]> * Improved manager.py, add doctest and safety mechanisms to plugin framework (timeout, memory cleanup, validation) Signed-off-by: Mihai Criveti <[email protected]> * Add README and renamed plugins with filter prefix Signed-off-by: Mihai Criveti <[email protected]> --------- Signed-off-by: Teryl Taylor <[email protected]> Signed-off-by: Frederico Araujo <[email protected]> Signed-off-by: Mihai Criveti <[email protected]> Co-authored-by: Teryl Taylor <[email protected]> Co-authored-by: Frederico Araujo <[email protected]> Co-authored-by: Mihai Criveti <[email protected]>
* feat: initial revision of adding plugin support. Signed-off-by: Teryl Taylor <[email protected]> * feat(plugins): added prompt posthook functionality with executor, fixed some linting issues, updated example plugin with posthook. Signed-off-by: Teryl Taylor <[email protected]> * feat(plugins): integrated plugins into prompt service, fixed linting and type issues. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): renamed types.py to plugin_types.py due to conflict in pytest Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): fixed renamed plugin_types module in prompt_service.py Signed-off-by: Teryl Taylor <[email protected]> * feat: add example filter plugin Signed-off-by: Frederico Araujo <[email protected]> * feat: add plugin violation error object Signed-off-by: Frederico Araujo <[email protected]> * feat: added unit tests Signed-off-by: Teryl Taylor <[email protected]> * fix(license): add licensing to unit test files and run lint. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugin): linting issues. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): added yaml dependency for plugins to pyproject.toml Signed-off-by: Teryl Taylor <[email protected]> * test(plugin): added tests for filter plugins Signed-off-by: Teryl Taylor <[email protected]> * test(plugin): add missing config files for plugin tests Signed-off-by: Teryl Taylor <[email protected]> * Add PII filter plugin Signed-off-by: Mihai Criveti <[email protected]> * docs(plugins): updated plugins documentation. Signed-off-by: Teryl Taylor <[email protected]> * fix: include plugin md files in manifest Signed-off-by: Frederico Araujo <[email protected]> * docs: add deny list plugin readme Signed-off-by: Frederico Araujo <[email protected]> * Pre-commit cleanup Signed-off-by: Mihai Criveti <[email protected]> * Improved manager.py, add doctest and safety mechanisms to plugin framework (timeout, memory cleanup, validation) Signed-off-by: Mihai Criveti <[email protected]> * Add README and renamed plugins with filter prefix Signed-off-by: Mihai Criveti <[email protected]> --------- Signed-off-by: Teryl Taylor <[email protected]> Signed-off-by: Frederico Araujo <[email protected]> Signed-off-by: Mihai Criveti <[email protected]> Co-authored-by: Teryl Taylor <[email protected]> Co-authored-by: Frederico Araujo <[email protected]> Co-authored-by: Mihai Criveti <[email protected]>
* feat: initial revision of adding plugin support. Signed-off-by: Teryl Taylor <[email protected]> * feat(plugins): added prompt posthook functionality with executor, fixed some linting issues, updated example plugin with posthook. Signed-off-by: Teryl Taylor <[email protected]> * feat(plugins): integrated plugins into prompt service, fixed linting and type issues. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): renamed types.py to plugin_types.py due to conflict in pytest Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): fixed renamed plugin_types module in prompt_service.py Signed-off-by: Teryl Taylor <[email protected]> * feat: add example filter plugin Signed-off-by: Frederico Araujo <[email protected]> * feat: add plugin violation error object Signed-off-by: Frederico Araujo <[email protected]> * feat: added unit tests Signed-off-by: Teryl Taylor <[email protected]> * fix(license): add licensing to unit test files and run lint. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugin): linting issues. Signed-off-by: Teryl Taylor <[email protected]> * fix(plugins): added yaml dependency for plugins to pyproject.toml Signed-off-by: Teryl Taylor <[email protected]> * test(plugin): added tests for filter plugins Signed-off-by: Teryl Taylor <[email protected]> * test(plugin): add missing config files for plugin tests Signed-off-by: Teryl Taylor <[email protected]> * Add PII filter plugin Signed-off-by: Mihai Criveti <[email protected]> * docs(plugins): updated plugins documentation. Signed-off-by: Teryl Taylor <[email protected]> * fix: include plugin md files in manifest Signed-off-by: Frederico Araujo <[email protected]> * docs: add deny list plugin readme Signed-off-by: Frederico Araujo <[email protected]> * Pre-commit cleanup Signed-off-by: Mihai Criveti <[email protected]> * Improved manager.py, add doctest and safety mechanisms to plugin framework (timeout, memory cleanup, validation) Signed-off-by: Mihai Criveti <[email protected]> * Add README and renamed plugins with filter prefix Signed-off-by: Mihai Criveti <[email protected]> --------- Signed-off-by: Teryl Taylor <[email protected]> Signed-off-by: Frederico Araujo <[email protected]> Signed-off-by: Mihai Criveti <[email protected]> Co-authored-by: Teryl Taylor <[email protected]> Co-authored-by: Frederico Araujo <[email protected]> Co-authored-by: Mihai Criveti <[email protected]>
β¨ Feature / Enhancement PR
π Epic / Issue
Closes #319
π Summary (1-2 sentences)
This PR adds plugin capabilities at special hook points throughout the MCP Context Forge to be able to perform AI safety, security, and business processing. More details in the issue.
π§ͺ Checks
make lintpassesmake testpasses