Skip to content

Commit 0567d0f

Browse files
committed
disallowedfor html&script
Signed-off-by: NAYANAR <[email protected]>
1 parent 35bad14 commit 0567d0f

File tree

5 files changed

+249
-36
lines changed

5 files changed

+249
-36
lines changed

mcpgateway/main.py

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -287,50 +287,78 @@ async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
287287

288288

289289
# Global exceptions handlers
290-
@app.exception_handler(RequestValidationError)
291-
async def validation_exception_handler(_request: Request, exc: RequestValidationError):
292-
"""Handle Pydantic validation errors globally.
293-
294-
Args:
295-
_request: The FastAPI request object that triggered the validation error.
296-
exc: The Pydantic ValidationError exception containing validation failure details.
297-
298-
Returns:
299-
JSONResponse: A 422 Unprocessable Entity response with formatted validation error details.
300-
"""
301-
return JSONResponse(status_code=422, content=ErrorFormatter.format_validation_error(exc))
302290

303291

304292
# Register exception handler for custom ValidationError
305293
@app.exception_handler(ValidationError)
306-
async def content_validation_exception_handler(_request: Request, exc: ValidationError):
307-
"""Handle content security validation errors with a plain message and no traceback.
294+
async def content_validation_exception_handler(request: Request, exc: ValidationError):
295+
"""Handle content security validation errors with a clean message format.
308296
309297
Args:
310-
_request: The FastAPI request object that triggered validation error.
298+
request: The FastAPI request object that triggered validation error.
311299
exc: The ValidationError exception containing failure details.
312300
313301
Returns:
314-
PlainTextResponse: Plain text error message with 400 status code.
302+
JSONResponse: Clean error message with 400 status code.
315303
"""
316-
return PlainTextResponse(f"mcpgateway.services.content_security.ValidationError: {exc}", status_code=400)
304+
# Determine the operation type from the request path
305+
path = request.url.path
306+
if "/resources" in path:
307+
operation = "register resource"
308+
elif "/prompts" in path:
309+
operation = "create prompt"
310+
elif "/tools" in path:
311+
operation = "create tool"
312+
else:
313+
operation = "process request"
314+
315+
# Extract the actual error message and clean it up
316+
error_msg = str(exc)
317+
if "Content contains HTML tags that may cause display issues" in error_msg:
318+
error_msg = "Resource content contains disallowed HTML tags"
319+
320+
return JSONResponse(
321+
status_code=400,
322+
content={"detail": f"Failed to {operation}: {error_msg}"}
323+
)
317324

318325

319326
@app.exception_handler(RequestValidationError)
320-
async def request_validation_exception_handler(_request: Request, exc: RequestValidationError):
327+
async def request_validation_exception_handler(request: Request, exc: RequestValidationError):
321328
"""Handle FastAPI request validation errors (automatic request parsing).
322329
323330
This handles ValidationErrors that occur during FastAPI's automatic request
324331
parsing before the request reaches your endpoint.
325332
326333
Args:
327-
_request: The FastAPI request object that triggered validation error.
334+
request: The FastAPI request object that triggered validation error.
328335
exc: The RequestValidationError exception containing failure details.
329336
330337
Returns:
331338
JSONResponse: A 422 Unprocessable Entity response with error details.
332339
"""
333-
if _request.url.path.startswith("/tools"):
340+
# Check if this is a resource creation request with content validation error
341+
if request.url.path.startswith("/resources") and request.method == "POST":
342+
for error in exc.errors():
343+
msg = error.get("msg", "")
344+
loc = error.get("loc", [])
345+
# Debug logging
346+
logger.debug(f"Validation error - loc: {loc}, msg: {msg}")
347+
if len(loc) >= 2 and loc[-2:] == ["body", "content"] and "HTML tags" in msg:
348+
# Provide user-friendly message for HTML content validation
349+
return JSONResponse(
350+
status_code=400,
351+
content={"detail": "Failed to register resource: Resource content contains disallowed HTML tags"}
352+
)
353+
elif len(loc) >= 2 and loc[-2:] == ["body", "content"] and "Content contains" in msg:
354+
# Extract the actual error message after "Value error, "
355+
clean_msg = msg.replace("Value error, ", "") if "Value error, " in msg else msg
356+
return JSONResponse(
357+
status_code=400,
358+
content={"detail": f"Failed to register resource: {clean_msg}"}
359+
)
360+
361+
if request.url.path.startswith("/tools"):
334362
error_details = []
335363

336364
for error in exc.errors():
@@ -348,7 +376,7 @@ async def request_validation_exception_handler(_request: Request, exc: RequestVa
348376

349377
response_content = {"detail": error_details}
350378
return JSONResponse(status_code=422, content=response_content)
351-
return await fastapi_default_validation_handler(_request, exc)
379+
return await fastapi_default_validation_handler(request, exc)
352380

353381

354382
@app.exception_handler(IntegrityError)
@@ -1586,15 +1614,15 @@ async def create_resource(
15861614
)
15871615
except SecurityError as e:
15881616
logger.warning(f"Security violation in resource creation by user {user}: {str(e)}")
1589-
raise HTTPException(status_code=400, detail="Content failed security validation")
1617+
raise HTTPException(status_code=400, detail=f"Failed to register resource: {str(e)}")
15901618
except ValidationError as e:
1591-
raise HTTPException(status_code=400, detail=str(e))
1619+
raise HTTPException(status_code=400, detail=f"Failed to register resource: {str(e)}")
15921620
except ResourceURIConflictError as e:
15931621
raise HTTPException(status_code=409, detail=str(e))
15941622
except ResourceError as e:
15951623
if "Rate limit" in str(e):
15961624
raise HTTPException(status_code=429, detail=str(e))
1597-
raise HTTPException(status_code=400, detail=str(e))
1625+
raise HTTPException(status_code=400, detail=f"Failed to register resource: {str(e)}")
15981626
except IntegrityError as e:
15991627
logger.error(f"Integrity error while creating resource: {e}")
16001628
raise HTTPException(status_code=409, detail=ErrorFormatter.format_database_error(e))

mcpgateway/schemas.py

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,11 +1111,12 @@ def validate_mime_type(cls, v: Optional[str]) -> Optional[str]:
11111111

11121112
@field_validator("content")
11131113
@classmethod
1114-
def validate_content(cls, v: Optional[Union[str, bytes]]) -> Optional[Union[str, bytes]]:
1114+
def validate_content(cls, v: Optional[Union[str, bytes]], info: ValidationInfo) -> Optional[Union[str, bytes]]:
11151115
"""Validate content size and safety
11161116
11171117
Args:
11181118
v (Union[str, bytes]): Value to validate
1119+
info (ValidationInfo): Validation context containing other field values
11191120
11201121
Returns:
11211122
Union[str, bytes]: Value if validated as safe
@@ -1137,12 +1138,18 @@ def validate_content(cls, v: Optional[Union[str, bytes]]) -> Optional[Union[str,
11371138
else:
11381139
text = v
11391140

1140-
# Skip HTML validation in test environment
1141+
# Get MIME type from validation context
1142+
mime_type = (info.data.get("mime_type") or "").lower() if info.data else ""
1143+
1144+
# Always block HTML content regardless of MIME type (except in tests)
11411145
if not os.environ.get("PYTEST_CURRENT_TEST") and re.search(SecurityValidator.DANGEROUS_HTML_PATTERN, text, re.IGNORECASE):
1142-
raise ValueError("Content contains HTML tags that may cause display issues")
1143-
1144-
# if re.search(SecurityValidator.DANGEROUS_HTML_PATTERN, text, re.IGNORECASE):
1145-
# raise ValueError("Content contains HTML tags that may cause display issues")
1146+
# Check for specific dangerous tags
1147+
if "<script" in text.lower():
1148+
raise ValueError("Resource content contains disallowed script tags")
1149+
elif "<html" in text.lower():
1150+
raise ValueError("Resource content contains disallowed HTML tags")
1151+
else:
1152+
raise ValueError("Resource content contains disallowed HTML tags")
11461153

11471154
return v
11481155

@@ -1225,11 +1232,12 @@ def validate_mime_type(cls, v: Optional[str]) -> Optional[str]:
12251232

12261233
@field_validator("content")
12271234
@classmethod
1228-
def validate_content(cls, v: Optional[Union[str, bytes]]) -> Optional[Union[str, bytes]]:
1235+
def validate_content(cls, v: Optional[Union[str, bytes]], info: ValidationInfo) -> Optional[Union[str, bytes]]:
12291236
"""Validate content size and safety
12301237
12311238
Args:
12321239
v (Union[str, bytes]): Value to validate
1240+
info (ValidationInfo): Validation context containing other field values
12331241
12341242
Returns:
12351243
Union[str, bytes]: Value if validated as safe
@@ -1250,9 +1258,19 @@ def validate_content(cls, v: Optional[Union[str, bytes]]) -> Optional[Union[str,
12501258
raise ValueError("Content must be UTF-8 decodable")
12511259
else:
12521260
text = v
1253-
# Skip HTML validation in test environment
1261+
1262+
# Get MIME type from validation context
1263+
mime_type = (info.data.get("mime_type") or "").lower() if info.data else ""
1264+
1265+
# Always block HTML content regardless of MIME type (except in tests)
12541266
if not os.environ.get("PYTEST_CURRENT_TEST") and re.search(SecurityValidator.DANGEROUS_HTML_PATTERN, text, re.IGNORECASE):
1255-
raise ValueError("Content contains HTML tags that may cause display issues")
1267+
# Check for specific dangerous tags
1268+
if "<script" in text.lower():
1269+
raise ValueError("Resource content contains disallowed script tags")
1270+
elif "<html" in text.lower():
1271+
raise ValueError("Resource content contains disallowed HTML tags")
1272+
else:
1273+
raise ValueError("Resource content contains disallowed HTML tags")
12561274

12571275
return v
12581276

mcpgateway/services/content_security.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,13 @@ async def _validate_content(self, content: str, _mime_type: str, context: str) -
171171
for pattern in self.dangerous_patterns:
172172
if pattern.search(content_lower):
173173
self.security_violations["dangerous_pattern"] += 1
174-
raise SecurityError(f"{context.capitalize()} content contains potentially dangerous pattern: {pattern.pattern}")
174+
# Check for specific script tags
175+
if "<script" in content_lower:
176+
raise SecurityError("Resource content contains disallowed script tags")
177+
elif "<html" in content_lower:
178+
raise SecurityError("Resource content contains disallowed HTML tags")
179+
else:
180+
raise SecurityError(f"Resource content contains disallowed {pattern.pattern} pattern")
175181
# Check for excessive whitespace (potential padding attack, only for text)
176182
if isinstance(content, str) and len(content) > 1000: # Only check larger content
177183
whitespace_ratio = sum(1 for c in content if c.isspace()) / len(content)

mcpgateway/services/resource_service.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ async def register_resource(
272272
lowered_content = resource.content.lower()
273273
for tag in disallowed_tags:
274274
if tag in lowered_content:
275-
raise ResourceError(f"Resource content contains disallowed {tag} tag")
275+
raise ResourceError(f"Resource content contains disallowed {tag.replace('<', '')} tags")
276276
validated_content, _ = await content_security.validate_resource_content(content=resource.content, uri=resource.uri, mime_type=resource.mime_type)
277277
resource.content = validated_content
278278
if not resource.mime_type:
@@ -289,7 +289,7 @@ async def register_resource(
289289

290290
# --- Enforce disallowed MIME types ---
291291
if mime_type in DISALLOWED_MIME_TYPES:
292-
raise ResourceError(f"MIME type '{mime_type}' is not allowed")
292+
raise ResourceError(f"Resource content contains disallowed MIME type '{mime_type}'")
293293

294294
# Determine content storage
295295
is_text = mime_type and mime_type.startswith("text/") or isinstance(resource.content, str)

test_resource_security.py

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Simple test script to verify resource security validation.
4+
This tests the specific scenarios mentioned in the user's curl commands.
5+
"""
6+
7+
import asyncio
8+
import sys
9+
import os
10+
11+
# Add the project root to Python path
12+
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
13+
14+
from mcpgateway.services.resource_service import ResourceService, ResourceError
15+
from mcpgateway.schemas import ResourceCreate
16+
from mcpgateway.config import settings
17+
from unittest.mock import MagicMock
18+
19+
async def test_script_injection():
20+
"""Test that script tags are blocked."""
21+
print("Testing script injection...")
22+
23+
service = ResourceService()
24+
25+
# Mock database session
26+
db = MagicMock()
27+
28+
# Test 1: Script injection
29+
try:
30+
resource = ResourceCreate(
31+
uri="test://script",
32+
name="Script",
33+
content="<script>alert(1)</script>"
34+
)
35+
36+
result = await service.register_resource(db, resource)
37+
print("❌ Script injection was NOT blocked!")
38+
return False
39+
except ResourceError as e:
40+
if "disallowed script tags" in str(e):
41+
print("✅ Script injection correctly blocked:", str(e))
42+
return True
43+
else:
44+
print("❌ Script injection blocked but with wrong message:", str(e))
45+
return False
46+
except Exception as e:
47+
print("❌ Unexpected error:", str(e))
48+
return False
49+
50+
async def test_html_mime_type():
51+
"""Test that HTML MIME type is blocked."""
52+
print("Testing HTML MIME type...")
53+
54+
service = ResourceService()
55+
56+
# Mock database session
57+
db = MagicMock()
58+
59+
# Test 2: HTML MIME type
60+
try:
61+
resource = ResourceCreate(
62+
uri="test.html",
63+
name="HTML",
64+
content="<html>test</html>",
65+
mime_type="text/html"
66+
)
67+
68+
result = await service.register_resource(db, resource)
69+
print("❌ HTML MIME type was NOT blocked!")
70+
return False
71+
except ResourceError as e:
72+
if "disallowed MIME type" in str(e) and "text/html" in str(e):
73+
print("✅ HTML MIME type correctly blocked:", str(e))
74+
return True
75+
else:
76+
print("❌ HTML MIME type blocked but with wrong message:", str(e))
77+
return False
78+
except Exception as e:
79+
print("❌ Unexpected error:", str(e))
80+
return False
81+
82+
async def test_valid_content():
83+
"""Test that valid content is allowed."""
84+
print("Testing valid content...")
85+
86+
service = ResourceService()
87+
88+
# Mock database session and its methods
89+
db = MagicMock()
90+
db.add = MagicMock()
91+
db.commit = MagicMock()
92+
db.refresh = MagicMock()
93+
94+
# Mock the resource object that would be created
95+
mock_resource = MagicMock()
96+
mock_resource.id = 1
97+
mock_resource.uri = "test://valid"
98+
mock_resource.name = "Valid"
99+
mock_resource.content = "This is valid content"
100+
mock_resource.metrics = []
101+
mock_resource.tags = []
102+
103+
# Make refresh set the mock resource
104+
def refresh_side_effect(resource):
105+
resource.id = 1
106+
resource.metrics = []
107+
resource.tags = []
108+
109+
db.refresh.side_effect = refresh_side_effect
110+
111+
try:
112+
resource = ResourceCreate(
113+
uri="test://valid",
114+
name="Valid",
115+
content="This is valid content"
116+
)
117+
118+
result = await service.register_resource(db, resource)
119+
print("✅ Valid content correctly allowed")
120+
return True
121+
except Exception as e:
122+
print("❌ Valid content was blocked:", str(e))
123+
return False
124+
125+
async def main():
126+
"""Run all tests."""
127+
print("Running resource security validation tests...\n")
128+
129+
# Enable content validation patterns for testing
130+
settings.content_validate_patterns = True
131+
132+
results = []
133+
134+
# Test script injection
135+
results.append(await test_script_injection())
136+
print()
137+
138+
# Test HTML MIME type
139+
results.append(await test_html_mime_type())
140+
print()
141+
142+
# Test valid content
143+
results.append(await test_valid_content())
144+
print()
145+
146+
# Summary
147+
passed = sum(results)
148+
total = len(results)
149+
150+
print(f"Results: {passed}/{total} tests passed")
151+
152+
if passed == total:
153+
print("✅ All security validation tests passed!")
154+
return 0
155+
else:
156+
print("❌ Some security validation tests failed!")
157+
return 1
158+
159+
if __name__ == "__main__":
160+
exit_code = asyncio.run(main())
161+
sys.exit(exit_code)

0 commit comments

Comments
 (0)