Skip to content

Commit 1b20616

Browse files
committed
fix: Address all PR feedback for Issue IBM#2226
1. Fix broken imports (Issue #1): - Change from ..database to ..db - Fix unified_pdp imports to use plugins.unified_pdp - Update in routes, services, schemas, and tests 2. Register sandbox router in main.py (Issue IBM#2): - Add import and app.include_router call 3. Fix XSS vulnerability (Issue IBM#3): - Replace f-string HTML with Jinja2 template - Create sandbox_simulate_results.html template - Add Request parameter for template access 4. Add authentication (Issue IBM#4): - Add Depends(get_current_user) to simulate endpoint 5. Remove scratch files (Issue IBM#5): - Delete sandbox_header.txt and sandbox_new_header.txt 6. Resolve schemas conflict (Issue IBM#6): - Merge schemas/sandbox.py into schemas.py - Remove conflicting schemas/ directory - Update imports in routes and services All changes tested and ready for review. Related to IBM#2226 Signed-off-by: hughhennelly <hughhennelly06@gmail.com>
1 parent d6d7a96 commit 1b20616

8 files changed

Lines changed: 462 additions & 501 deletions

File tree

mcpgateway/main.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
from mcpgateway.plugins.framework import PluginError, PluginManager, PluginViolationError
8989
from mcpgateway.routers.server_well_known import router as server_well_known_router
9090
from mcpgateway.routers.well_known import router as well_known_router
91+
from mcpgateway.routes.sandbox import router as sandbox_router
9192
from mcpgateway.schemas import (
9293
A2AAgentCreate,
9394
A2AAgentRead,
@@ -7320,6 +7321,7 @@ async def cleanup_import_statuses(max_age_hours: int = 24, user=Depends(get_curr
73207321
# Conditional UI and admin API handling
73217322
if ADMIN_API_ENABLED:
73227323
logger.info("Including admin_router - Admin API enabled")
7324+
app.include_router(sandbox_router, prefix="/api/sandbox", tags=["Sandbox"])
73237325
app.include_router(admin_router) # Admin routes imported from admin.py
73247326
else:
73257327
logger.warning("Admin API routes not mounted - Admin API disabled via MCPGATEWAY_ADMIN_API_ENABLED=False")

mcpgateway/routes/sandbox.py

Lines changed: 10 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@
2020
from typing import Optional
2121

2222
# Third-Party
23-
from fastapi import APIRouter, Depends, Form, HTTPException, status
23+
from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
24+
from mcpgateway.auth import get_current_user
2425
from fastapi.responses import HTMLResponse
2526
from sqlalchemy.orm import Session
2627

2728
# Local
2829
from ..db import get_db
29-
from ..schemas.sandbox import (
30+
from ..schemas import (
3031
BatchSimulateRequest,
3132
BatchSimulationResult,
3233
RegressionReport,
@@ -505,6 +506,8 @@ async def service_info() -> dict:
505506

506507
@router.post("/sandbox/simulate", response_class=HTMLResponse)
507508
async def simulate_form_submit(
509+
request: Request,
510+
current_user=Depends(get_current_user),
508511
policy_draft_id: str = Form(...),
509512
subject_email: str = Form(...),
510513
subject_roles: str = Form(...),
@@ -554,96 +557,13 @@ async def simulate_form_submit(
554557
include_explanation=True,
555558
)
556559

557-
# Generate HTML response
558-
passed_class = "bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-200" if result.passed else "bg-red-100 text-red-800 dark:bg-red-900 dark:text-red-200"
559-
passed_text = "PASSED ✓" if result.passed else "FAILED ✗"
560-
561-
policies_html = "".join(
562-
[
563-
f'<span class="inline-flex items-center px-2.5 py-0.5 rounded-full text-xs font-medium bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-200">{policy}</span>'
564-
for policy in result.matching_policies
565-
]
560+
# Render template response with auto-escaping
561+
return request.app.state.templates.TemplateResponse(
562+
request,
563+
"sandbox_simulate_results.html",
564+
{"result": result}
566565
)
567566

568-
explanation_html = ""
569-
if result.explanation:
570-
explanation_html = f"""
571-
<div class="border-t border-gray-200 dark:border-gray-700 pt-6">
572-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400 mb-3">Detailed Explanation</h4>
573-
<div class="bg-gray-50 dark:bg-gray-900 rounded-md p-4">
574-
<pre class="text-sm text-gray-700 dark:text-gray-300 whitespace-pre-wrap font-mono">{result.explanation}</pre>
575-
</div>
576-
</div>
577-
"""
578-
579-
html = f"""
580-
<div class="bg-white dark:bg-gray-800 shadow rounded-lg">
581-
<div class="px-6 py-4 border-b border-gray-200 dark:border-gray-700">
582-
<h3 class="text-lg font-semibold text-gray-900 dark:text-white">
583-
Simulation Results
584-
</h3>
585-
</div>
586-
587-
<div class="p-6 space-y-6">
588-
<!-- Result Badge -->
589-
<div class="flex items-center justify-between">
590-
<div>
591-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400">Test Result</h4>
592-
<div class="mt-2">
593-
<span class="px-3 py-1 inline-flex text-sm leading-5 font-semibold rounded-full {passed_class}">
594-
{passed_text}
595-
</span>
596-
</div>
597-
</div>
598-
<div class="text-right">
599-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400">Execution Time</h4>
600-
<p class="mt-2 text-2xl font-bold text-gray-900 dark:text-white">
601-
{result.execution_time_ms}ms
602-
</p>
603-
</div>
604-
</div>
605-
606-
<!-- Decision Details -->
607-
<div class="grid grid-cols-1 md:grid-cols-2 gap-6 border-t border-gray-200 dark:border-gray-700 pt-6">
608-
<div>
609-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400 mb-2">Expected Decision</h4>
610-
<p class="text-lg font-semibold text-gray-900 dark:text-white">
611-
{result.expected_decision.value}
612-
</p>
613-
</div>
614-
<div>
615-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400 mb-2">Actual Decision</h4>
616-
<p class="text-lg font-semibold text-gray-900 dark:text-white">
617-
{result.actual_decision.value}
618-
</p>
619-
</div>
620-
</div>
621-
622-
<!-- Matching Policies -->
623-
<div class="border-t border-gray-200 dark:border-gray-700 pt-6">
624-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400 mb-3">Matching Policies</h4>
625-
<div class="space-x-2 space-y-2">
626-
{policies_html if policies_html else '<p class="text-sm text-gray-500 dark:text-gray-400">No policies matched</p>'}
627-
</div>
628-
</div>
629-
630-
<!-- Reason -->
631-
<div class="border-t border-gray-200 dark:border-gray-700 pt-6">
632-
<h4 class="text-sm font-medium text-gray-500 dark:text-gray-400 mb-3">Decision Reason</h4>
633-
<div class="bg-gray-50 dark:bg-gray-900 rounded-md p-4">
634-
<p class="text-sm text-gray-700 dark:text-gray-300">
635-
{result.reason}
636-
</p>
637-
</div>
638-
</div>
639-
640-
{explanation_html}
641-
</div>
642-
</div>
643-
"""
644-
645-
return HTMLResponse(content=html)
646-
647567
except Exception as e:
648568
logger.exception("Error running simulation")
649569
error_html = f"""

0 commit comments

Comments
 (0)