Skip to content

Conversation

madhav165
Copy link
Collaborator

@madhav165 madhav165 commented Jul 10, 2025

🐛 Bug-fix PR

Closes #339 (/admin endpoints), Closes #340 (all other endpoints)

Related defects on JavaScript /UI code are closed by separate PRs: #337 (Add proper HTML escaping for admin UI) and #338 (resolve all lint issues in web stack)

📌 Summary

What problem does this PR fix and why?

This PR implements comprehensive input validation for all API endpoints to prevent XSS, injection attacks, and data integrity issues. User-controlled data that gets displayed in the UI can cause layout problems, security vulnerabilities, and unexpected behavior without proper validation and escaping. This fix ensures all user input is validated against strict patterns and sanitized before storage and display.

🔁 Reproduction Steps

Link the issue and minimal steps to reproduce the bug.

Issues: #339, #340

  1. Send malicious payloads to any API endpoint (e.g., POST /admin/tools with name: <script>alert(1)</script>)
  2. Observe that without validation, these payloads are stored and rendered in the UI
  3. This can break UI layouts or execute unwanted scripts

🐞 Root Cause

What was wrong and where?

API endpoints were accepting and storing user input without validation, allowing:

  • HTML/script injection through fields like names, descriptions, and templates
  • Malformed URLs and URIs
  • Excessively large payloads
  • Invalid MIME types and JSON structures

💡 Fix Description

How did you solve it? Key design points.

  1. Created centralized SecurityValidator class (mcpgateway/validators.py):

    • Pattern-based validation for dangerous HTML/JS content
    • Field-specific validators (names, URLs, templates, etc.)
    • Configurable limits via settings
    • HTML escaping for display text
  2. Applied validators to all Pydantic schemas:

    • Enhanced ToolCreate, ResourceCreate, PromptCreate, GatewayCreate, ServerCreate
    • Added field validators using @field_validator decorators
    • Consistent error messages for better UX
  3. Configuration-driven approach:

    • All patterns and limits configurable via settings
    • MCP-compliant defaults (e.g., 255 char names, 4KB descriptions)

🧪 Verification

Check Command Status
Unit tests make test ✅ pass
New validator tests pytest tests/unit/mcpgateway/validation/test_validators.py ✅ pass

📐 MCP Compliance (if relevant)

  • Matches current MCP spec (tool naming, URI patterns)
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed
  • Added comprehensive test coverage for validators
  • All existing tests pass with new validation

Signed-off-by: Madhav Kandukuri <[email protected]>
@madhav165 madhav165 marked this pull request as draft July 10, 2025 11:01
Signed-off-by: Madhav Kandukuri <[email protected]>
@madhav165 madhav165 linked an issue Jul 10, 2025 that may be closed by this pull request
7 tasks
Signed-off-by: Madhav Kandukuri <[email protected]>
Signed-off-by: Madhav Kandukuri <[email protected]>
Signed-off-by: Madhav Kandukuri <[email protected]>
Signed-off-by: Madhav Kandukuri <[email protected]>
Signed-off-by: Madhav Kandukuri <[email protected]>
@madhav165
Copy link
Collaborator Author

madhav165 commented Jul 10, 2025

Tested with

curl --request POST \
  --url http://localhost:4444/gateways \
  --header 'Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN
  --header 'content-type: application/json' \
  --data '{
  "name": "g7@LaPX#8qcz2MUEYwK(0R^4tnJZidBb+ol5DFVeN[Wpm93A1hI{O*}xG6vCTHsSQkfj!ry]-u=|>L8Z`$aXz(mY+B#R5c92nPUVW%Jd0MEhxA>oGwfqNevKg3s^F[Ht@L1bC)!=j4}TDQpIMlSZuNKOGm7~yRxJ9Bv+W>XizCkf(nlY&82#oqr5PA$JU}a*M-Z=@wEgphdL3VKI]CtNYX^69bmfT{+es0!u7~FWrOHv1LRydGC2qx]jz#n<BkDMU@8V(PZ&%)aA$T5hXowmiEgYl!J^bfM=NQcu7StdKCrx{4I}-vO3p9Bn+LzWYkjPQe@Hm%NXI!ow2^vTuCc5z#RYg9Bh(03LdaP=F&bZUJ-E+n4x$NKrsK{1t)V8MyidGLqj7AQhCmWR^pOs6ewXF2nlUYz!#@g0}93b&dT5K+%vH=[INBOMZra)*yLqxEJpCWfUhoR7twYnm+VX*ikgtUZP@#LAd1&cw29H^qOjbs5eyR~KlFC63MVnXpG%uWTdN(B!m+=rzJY4{aoE}-x7I9lf^UQKT5Xyw3C$gBAOHpN0RJqEk7dPGn4vztMbXCl%V!&L[uWYrosTI@9j=1ZKh3fxca}+-5NOM^PYUgFEG!bpLXqHd7Am]T#o*KWn{u0CJrszv2yt&934VXUIljMC",
  "url": "http://localhost:8000/sse",
  "transport_type": "SSE"
}'
curl --request POST \
  --url http://localhost:4444/gateways \
  --header 'Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN' \
  --header 'content-type: application/json' \
  --data '{
  "name": "test",
  "url": "hello_world",
  "transport_type": "SSE"
}'
curl --request POST \
  --url http://localhost:4444/gateways \
  --header 'Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN' \
  --header 'content-type: application/json' \
  --data '{
  "name": "<script>",
  "url": "hello_world",
  "transport_type": "SSE"
}'
curl --request POST \
  --url http://localhost:4444/tools \
  --header 'Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN' \
  --header 'content-type: application/json' \
  --data '{
  "name": "<script>",
  "url": "hello_world",
  "integration_type": "REST",
  "request_type": "GET"
}'

@madhav165 madhav165 marked this pull request as ready for review July 10, 2025 12:52
@madhav165 madhav165 changed the title Validate inputs to admin APIs Validate inputs to admin and main APIs Jul 10, 2025
@crivetimihai crivetimihai added the security Improves security label Jul 10, 2025
@crivetimihai crivetimihai self-assigned this Jul 10, 2025
@crivetimihai crivetimihai added this to the Release 0.4.0 milestone Jul 10, 2025
@crivetimihai
Copy link
Member

crivetimihai commented Jul 10, 2025

Testing Checklist for Input Validation PR

🔧 Setup & Basic Functionality

  • make venv install install-dev serve
  • make serve
  • Verify server starts without errors on http://localhost:4444[x] make test - All existing tests pass
  • make lint - No new linting issues
  • make tomllint jsonlint
  • make smoketest container build based smoketest passes
  • make docker-prod docker-run-ssl-host # container works
  • curl -sk https://localhost:4444/health
  • curl -sk https://localhost:4444/ready
  • export MCPGATEWAY_BEARER_TOKEN=$(python3 -m mcpgateway.utils.create_jwt_token -u admin --secret my-test-key)
  • curl -sk -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" https://localhost:4444/version
  • make docker-stop compose-down compose-rm compose-up
  • curl -sk -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" http://localhost:4444/version
  • http://localhost:4444/admin/ - still has existing tools, tested all tabs

Issue with "Test Server Connectivity" UI: ❌ Error: Invalid URL: URL is required

🧪 Test Valid Inputs (Happy Path) ❌

Tools: Create a new tool from a REST API

Create MCP Tool

  • Create tool with:
    • Name: mcp_tool_test
    • URL: https://example.com/mcp
    • Description: This is a helpful MCP tool for processing data
    • Integration Type: MCP
    • Request Type: SSE
    • Headers: {"Content-Type": "application/json"}
    • Input Schema: {"type": "object", "properties": {"input": {"type": "string"}}}

❌ nothing happened. Though we should remove adding SSE tools as this is not supported.

Create REST API Tool

  • Create tool with:
    • Name: my_tool_123
    • URL: https://example.com/api
    • Description: REST API tool for external service integration
    • Integration Type: REST
    • Request Type: POST
    • Headers: {"Authorization": "Bearer token", "Content-Type": "application/json"}
    • Input Schema: {"type": "object", "properties": {"query": {"type": "string"}, "limit": {"type": "number"}}}

❌ 1 validation error for ToolCreate request_type Value error, Request type 'POST' not allowed for MCP integration [type=value_error, input_value='POST', input_type=str] For further information visit https://errors.pydantic.dev/2.11/v/value_error

Update Tool

  • Update existing tool with:
    • Description: This is an updated helpful tool for processing complex data workflows
    • Add Annotation: {"title": "Updated Tool", "destructiveHint": false}

Resources

  • Create resource with:

    • URI: docs/guide (simple path, no file://)
    • Name: user_guide
    • Description: Comprehensive user guide for the application
    • MIME Type: text/markdown
    • Content: # User Guide\n\nWelcome to our application!\n\n## Getting Started\n1. First step\n2. Second step
  • Create JSON resource with:

    • URI: config/settings
    • Name: app_config
    • Description: Application configuration settings
    • MIME Type: application/json
    • Content: {"version": "1.0.0", "features": {"auth": true, "logging": true}, "maxUsers": 100}

❌ UI returns: Connection failed!

  • Update resource with:
    • Content: # Updated User Guide\n\nWelcome to version 2.0!\n\n## New Features\n- Feature 1\n- Feature 2
    • Description: Updated guide with new features

Prompts

  • Create prompt with:
    • Name: greeting_prompt
    • Description: Personalized greeting template with conditional logic
    • Template: Hello {{ name }}, welcome to {{ company }}!\n{% if role %}Your role is: {{ role }}{% endif %}\n{% if department %}Department: {{ department }}{% endif %}
    • Arguments:
      1. Argument 1: Name=name, Description=User's full name, Type=string, Required=true
      2. Argument 2: Name=company, Description=Company name, Type=string, Required=true
      3. Argument 3: Name=role, Description=User's job role, Type=string, Required=false
      4. Argument 4: Name=department, Description=Department name, Type=string, Required=false

Gateways (MCP Servers)

Setup: Run the fast-time-server: docker run --rm -it -p 8888:8080 ghcr.io/ibm/fast-time-server:latest -transport=dual --port 8002

  • Create gateway (MCP Server) with SSE:

    • Name: time_server
    • URL: http://127.0.0.1:8002
    • Description: Time server (MCP)
    • Transport: SSE
  • Create gateway (MCP Server) with HTTP:

    • Name: time_server
    • URL: http://127.0.0.1:8002/http
    • Description: Time server (MCP)
    • Transport: HTTP

Gateways (MCP Servers) with auth

Setup: Run the fast-time-server: docker run --rm -it -p 8888:8080 ghcr.io/ibm/fast-time-server:latest -transport=dual --port 8002 -auth-token=secret123

  • Create gateway (MCP Server) with SSE:

    • Name: time_server
    • URL: http://127.0.0.1:8002/sse
    • Description: Time server (MCP)
    • Transport: SSE
    • Auth: Bearer secret123
  • Create gateway (MCP Server) with HTTP:

    • Name: time_server
    • URL: http://127.0.0.1:8002/http
    • Description: Time server (MCP)
    • Transport: HTTP
    • Auth: Bearer secret123

Servers

  • Create server with:
    • Name: github
    • Description: My GitHub integration server for repository management
    • Icon URL: https://github.githubassets.com/assets/GitHub-Mark-ea2971cee799.png
    • Associated Tools: Select 2-3 existing tools from dropdown
    • Associated Resources: Select 1-2 existing resources from dropdown
    • Associated Prompts: Select greeting_prompt from dropdown

🚫 Test Invalid Inputs (Security Validation)

XSS/HTML Injection (test via Ui and API)

  • Try creating tool with name: <script>alert(1)</script> → Should fail
  • Try description with: <iframe src="evil.com"></iframe> → Should fail
  • Try template with: <form action="steal.php"> → Should fail
  • Try URL with: javascript:alert(1) → Should fail

SQL Injection Patterns

  • Try name with: '; DROP TABLE tools; -- → Should fail with "Special characters not allowed"
  • Try URI with: ../../etc/passwd → Should fail with "directory traversal"

Length Limits

  • Try name over 255 chars → Should fail with length error
  • Try description over 4096 chars → Should fail with length error
  • Try URL over 2048 chars → Should fail with length error

Invalid Formats

  • Try tool name starting with number: 123_tool → Should fail
  • Try invalid MIME type: not/a/mime → Should fail
  • Try invalid URL: ftp://server.com → Should fail (not in allowed schemes)

📊 API Testing with curl/httpie

Test Admin Endpoints

# Should fail - XSS attempt
curl -X POST http://localhost:8000/admin/tools \
  -H "Content-Type: application/json" \
  -d '{"name": "<script>alert(1)</script>", "url": "https://example.com"}'

# Should succeed - valid input
curl -X POST http://localhost:8000/admin/tools \
  -H "Content-Type: application/json" \
  -d '{"name": "valid_tool", "url": "https://example.com", "description": "A safe tool"}'

Test Main API Endpoints

# Test resource creation with invalid URI
curl -X POST http://localhost:8000/resources \
  -H "Content-Type: application/json" \
  -d '{"uri": "bad<uri>", "name": "test", "content": "data"}'

# Test prompt with dangerous template
curl -X POST http://localhost:8000/prompts \
  -H "Content-Type: application/json" \
  -d '{"name": "test", "template": "<script>evil()</script>"}'

🎯 Edge Cases

  • Test with empty strings for required fields → Should fail with "cannot be empty"
  • Test with only whitespace → Should be stripped and fail if required
  • Test Unicode characters in names → Should work if alphanumeric
  • Test nested JSON depth over 10 levels → Should fail with depth error
  • Test mixed valid/invalid data in batch operations

🔍 Regression Testing

  • Existing tools/resources/prompts still load correctly
  • Can still update existing entities with valid data
  • API responses include helpful validation error messages
  • No performance degradation (< 10ms per request as per spec)

📝 Documentation & Logs

  • make docs - Documentation builds without errors
  • Check logs for any new warnings/errors during validation
  • Verify error messages are user-friendly and don't leak internals

🚀 Final Verification

  • make pre-commit - All pre-commit hooks pass
  • make ci - Full CI pipeline passes locally
  • make clean build - Can build fresh from scratch
  • Test with both make serve and make serve-prod

🎭 Browser Testing (if admin UI enabled)

  • Navigate admin UI - all pages load correctly
  • Existing data displays properly (HTML entities escaped)
  • Form validation shows user-friendly errors
  • No console errors in browser dev tools

✅ Smoke Test Complete Workflow

  1. Start fresh: make clean-all && make venv install install-dev
  2. Run migrations: make migrate
  3. Start server: make serve
  4. Create valid tool → gateway → server → resource → prompt chain
  5. Verify all created entities work together
  6. Stop server gracefully with Ctrl+C

Kubernetes / Minikube Deployment / ArgoCD

  • Helm

@crivetimihai
Copy link
Member

crivetimihai commented Jul 10, 2025

📋 Post-Deployment Testing Checklist

🏥 Initial Health Checks

  • curl -sk https://localhost:4444/health | jq '.status' - Health check returns "healthy"
  • curl -sk https://localhost:4444/ready | jq '.ready' - Readiness check returns true
  • curl -sk -o /dev/null -w "%{http_code}" https://localhost:4444/health - Returns 200

🔑 Authentication Setup

  • export MCPGATEWAY_BEARER_TOKEN=$(python3 -m mcpgateway.utils.create_jwt_token -u admin --secret my-test-key)
  • curl -sk -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" https://localhost:4444/version | jq - Version with auth works

📊 API Verification - List Endpoints

  • curl -sk https://localhost:4444/tools | jq '.total' - Lists tools
  • curl -sk https://localhost:4444/resources | jq '.total' - Lists resources
  • curl -sk https://localhost:4444/prompts | jq '.total' - Lists prompts
  • curl -sk https://localhost:4444/gateways | jq '.total' - Lists gateways
  • curl -sk https://localhost:4444/servers | jq '.total' - Lists servers
  • curl -sk https://localhost:4444/admin/tools | jq '.total' - Admin tools endpoint
  • curl -sk https://localhost:4444/admin/resources | jq '.total' - Admin resources endpoint
  • curl -sk https://localhost:4444/admin/prompts | jq '.total' - Admin prompts endpoint

📚 OpenAPI & Documentation

  • curl -sk https://localhost:4444/openapi.json | jq '.info.title' - Returns "MCP Gateway"
  • curl -sk https://localhost:4444/docs | grep -q "MCP Gateway" && echo "✓ Swagger UI loads"
  • curl -sk https://localhost:4444/redoc | grep -q "ReDoc" && echo "✓ ReDoc loads"

🧪 Create Test Data via UI/API

Tools - MCP Tool

  • Navigate to https://localhost:4444/admin/ > Tools > Create New Tool
  • Create MCP tool with:
    • Name: mcp_tool_test
    • URL: https://example.com/mcp
    • Description: Test MCP tool for validation
    • Integration Type: MCP
    • Request Type: SSE
    • Headers: {"Content-Type": "application/json"}
    • Input Schema: {"type": "object", "properties": {"query": {"type": "string"}}}
  • Verify creation: curl -sk https://localhost:4444/tools/mcp_tool_test | jq '.name'

Tools - REST API Tool

  • Create REST tool with:
    • Name: rest_api_tool
    • URL: https://api.example.com/v1/endpoint
    • Description: REST API integration tool
    • Integration Type: REST
    • Request Type: POST
    • Headers: {"Authorization": "Bearer token"}
    • Input Schema: {"type": "object", "properties": {"data": {"type": "string"}}}
  • Verify creation: curl -sk https://localhost:4444/tools/rest_api_tool | jq '.integration_type' - Should show "REST"

Resources

  • Navigate to Resources > Create New Resource
  • Create markdown resource:
    • URI: docs/guide
    • Name: user_guide
    • Description: User guide for the application
    • MIME Type: text/markdown
    • Content: # User Guide\nThis is a simple guide for users.\n## Getting Started\nFollow these steps...
  • Create JSON resource:
    • URI: config/settings
    • Name: app_settings
    • Description: Application configuration
    • MIME Type: application/json
    • Content: {"version": "1.0", "features": {"auth": true}}
  • Verify: curl -sk https://localhost:4444/resources | jq '.items[].name' - Should include both

Prompts

  • Navigate to Prompts > Create New Prompt
  • Create prompt:
    • Name: greeting_prompt
    • Description: Personalized greeting template
    • Template: Hello {{ name }}, welcome to {{ company }}!\n{% if role %}Your role is: {{ role }}{% endif %}
    • Arguments:
      1. Name: name, Description: User's name, Type: string, Required: ✓
      2. Name: company, Description: Company name, Type: string, Required: ✓
      3. Name: role, Description: User's role, Type: string, Required: ✗
  • Test prompt execution:
    curl -sk -X POST https://localhost:4444/prompts/greeting_prompt \
      -H "Content-Type: application/json" \
      -d '{"args": {"name": "Alice", "company": "Acme Corp", "role": "Developer"}}' | jq
    Should return: "Hello Alice, welcome to Acme Corp! Your role is: Developer"

Gateways

  • Navigate to Gateways > Create New Gateway
  • Create gateway:
    • Name: main_gateway
    • URL: wss://gateway.example.com/ws
    • Description: Main WebSocket gateway for MCP
    • Transport: SSE
  • Verify: curl -sk https://localhost:4444/gateways/main_gateway | jq '.transport' - Should show "SSE"

Servers

  • Navigate to Servers > Create New Server
  • Create server:
    • Name: github_server
    • Description: GitHub integration server
    • Icon URL: https://github.githubassets.com/assets/GitHub-Mark-ea2971cee799.png
    • Associated Tools: Select mcp_tool_test and rest_api_tool
    • Associated Resources: Select user_guide
    • Associated Prompts: Select greeting_prompt
  • Verify: curl -sk https://localhost:4444/servers/github_server | jq '.icon' - Should show icon URL

❌ Error Handling Verification

  • curl -sk https://localhost:4444/tools/nonexistent | jq '.detail' - Returns 404 "Tool not found"
  • curl -sk -X POST https://localhost:4444/tools -H "Content-Type: application/json" -d '{}' | jq '.detail[0].msg' - 422 validation error
  • curl -sk https://localhost:4444/invalid-endpoint | jq '.detail' - 404 "Not Found"

🗄️ Database & Migrations

  • make db-current - Shows current revision (no pending migrations)
  • sqlite3 data/mcpgateway.db ".tables" | grep -E "(tools|resources|prompts)" | wc -l - Should show 3+ tables
  • sqlite3 data/mcpgateway.db "SELECT COUNT(*) FROM tools;" | grep -E "[0-9]+" - Has tools

📝 Log Verification

  • tail -n 20 logs/mcpgateway.log | grep -c ERROR - Should be 0
  • grep "Application startup complete" logs/mcpgateway.log | tail -1 - Startup logged
  • grep "Created.*tool.*mcp_tool_test" logs/mcpgateway.log - Tool creation logged

🔍 Process & Port Checks

  • lsof -i :4444 | grep LISTEN | wc -l - Port 4444 is listening (should be ≥1)
  • ps aux | grep "[m]cpgateway.server" | wc -l - Process running (should be ≥1)
  • netstat -tuln | grep 4444 - Shows listening on port 4444

🐳 Container Health (if using Docker)

  • docker ps --filter name=mcpgateway --format "table {{.Names}}\t{{.Status}}" | grep healthy - Container healthy
  • docker logs mcpgateway 2>&1 | tail -10 | grep -c ERROR - Should be 0
  • docker exec mcpgateway curl -s localhost:8000/health | jq '.status' - Internal health "healthy"
  • docker inspect mcpgateway | jq '.[0].State.Health.Status' - Should be "healthy"

✅ Final Integration Test

  • List all created entities:
    echo "=== Created Entities ==="
    echo "Tools: $(curl -sk https://localhost:4444/tools | jq -r '.items[] | select(.name | test("mcp_tool_test|rest_api_tool")) | .name' | tr '\n' ', ')"
    echo "Resources: $(curl -sk https://localhost:4444/resources | jq -r '.items[] | select(.name | test("user_guide|app_settings")) | .name' | tr '\n' ', ')"
    echo "Prompts: $(curl -sk https://localhost:4444/prompts | jq -r '.items[] | select(.name == "greeting_prompt") | .name')"
    echo "Gateways: $(curl -sk https://localhost:4444/gateways | jq -r '.items[] | select(.name == "main_gateway") | .name')"
    echo "Servers: $(curl -sk https://localhost:4444/servers | jq -r '.items[] | select(.name == "github_server") | .name')"
  • All entities should be listed and active

🧹 Cleanup (Optional)

  • Delete test data if needed:
    curl -sk -X DELETE https://localhost:4444/tools/mcp_tool_test
    curl -sk -X DELETE https://localhost:4444/tools/rest_api_tool
    curl -sk -X DELETE https://localhost:4444/prompts/greeting_prompt
    curl -sk -X DELETE https://localhost:4444/servers/github_server

Signed-off-by: Madhav Kandukuri <[email protected]>
@crivetimihai
Copy link
Member

New update passes test / smoketest.

Will merge this PR. Then we'll work on remaining issues in a separate defect.

@crivetimihai crivetimihai merged commit 7a31a25 into main Jul 10, 2025
18 of 19 checks passed
@crivetimihai crivetimihai deleted the validate-admin-inputs branch July 10, 2025 15:48
@madhav165 madhav165 restored the validate-admin-inputs branch July 10, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Improves security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security]: Add input validation for main API endpoints (depends on #339 /admin API validation) [Security]: Add input validation for /admin endpoints
2 participants