🛡️ Sentinel: [HIGH] Fix Information Leakage in API Errors

.jules/sentinel.md

@@ -7,3 +7,8 @@
 **Vulnerability:** Not a direct security vulnerability, but an environmental instability. The `requirements.txt` allowed `mistralai>=1.1.0`, which pulled in version 2.x.
 **Learning:** MistralAI 2.x introduces breaking changes in the client import structure (`from mistralai import Mistral` fails if not using the new client correctly or if expecting the old one). This caused the entire application (including security tests) to fail on startup.
 **Prevention:** Pin critical dependencies like `mistralai==1.1.0` in `requirements.txt` to ensure consistent behavior across development and CI environments, especially when using agents that rely on specific API structures.
+
+## 2025-05-15 - Global Exception Handler & Information Leakage
+**Vulnerability:** API endpoints were explicitly catching exceptions and returning `str(e)` in 500 responses, potentially leaking sensitive internal state (e.g., connection strings, file paths).
+**Learning:** Centralizing error handling with `@app.exception_handler(Exception)` allows for safe, generic client responses while maintaining detailed server-side logs. However, one must explicitly handle `StarletteHTTPException` and `RequestValidationError` within the global handler to avoid swallowing legitimate 4xx errors or validation details.
+**Prevention:** Avoid `try...except` blocks that return raw exception strings. Use a global exception handler and ensure it delegates or re-implements handling for built-in FastAPI exceptions to preserve API contract.

agents/mistral_agent/agent.py

@@ -300,47 +300,38 @@ class QuestionRequest(BaseModel):
 @router.post("/gap-analysis", summary="Analyze CMMC control gap with Mistral AI")
 async def gap_analysis(req: GapAnalysisRequest, db: AsyncSession = Depends(get_db)):
     """Use Mistral to analyze a compliance gap and return remediation steps."""
-    try:
-        result = await agent.analyze_gap(
-            req.control_id,
-            req.control_title,
-            req.control_description,
-            req.zt_pillar,
-            req.current_status,
-            req.existing_evidence,
-        )
-        await agent.record_run(
-            db, "manual", f"Gap Analysis: {req.control_id}", [req.control_id], result
-        )
-        return {
-            "control_id": req.control_id,
-            "analysis": result,
-            "model": MISTRAL_MODEL,
-        }
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
+    result = await agent.analyze_gap(
+        req.control_id,
+        req.control_title,
+        req.control_description,
+        req.zt_pillar,
+        req.current_status,
+        req.existing_evidence,
+    )
+    await agent.record_run(
+        db, "manual", f"Gap Analysis: {req.control_id}", [req.control_id], result
+    )
+    return {
+        "control_id": req.control_id,
+        "analysis": result,
+        "model": MISTRAL_MODEL,
+    }
 
 
 @router.post("/code-review", summary="DevSecOps code security analysis with Codestral")
 async def code_review(req: CodeReviewRequest, db: AsyncSession = Depends(get_db)):
     """Use Codestral to analyze code for CMMC-mapped security issues."""
-    try:
-        result = await agent.analyze_code_security(
-            req.code_snippet, req.language, req.relevant_controls
-        )
-        await agent.record_run(
-            db, "manual", "Code Review", req.relevant_controls or [], result
-        )
-        return {"analysis": result, "model": MISTRAL_CODE_MODEL}
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
+    result = await agent.analyze_code_security(
+        req.code_snippet, req.language, req.relevant_controls
+    )
+    await agent.record_run(
+        db, "manual", "Code Review", req.relevant_controls or [], result
+    )
+    return {"analysis": result, "model": MISTRAL_CODE_MODEL}
 
 
 @router.post("/ask", summary="Ask a CMMC/ZT compliance question")
 async def ask_question(req: QuestionRequest):
     """Natural language CMMC compliance Q&A powered by Mistral."""
-    try:
-        answer = await agent.answer_compliance_question(req.question, req.context)
-        return {"question": req.question, "answer": answer, "model": MISTRAL_MODEL}
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
+    answer = await agent.answer_compliance_question(req.question, req.context)
+    return {"question": req.question, "answer": answer, "model": MISTRAL_MODEL}

backend/main.py

@@ -8,12 +8,16 @@
 """
 
 import json
+import logging
 import os
 from contextlib import asynccontextmanager
 
 from dotenv import load_dotenv
-from fastapi import FastAPI
+from fastapi import FastAPI, Request
+from fastapi.exceptions import RequestValidationError
 from fastapi.middleware.cors import CORSMiddleware
+from fastapi.responses import JSONResponse
+from starlette.exceptions import HTTPException as StarletteHTTPException
 from fastapi_mcp import FastApiMCP
 
 from agents.devsecops_agent import agent as devsecops
@@ -34,6 +38,14 @@ async def lifespan(app: FastAPI):
     yield
 
 
+# ─── Logging ──────────────────────────────────────────────────────────────────
+
+logging.basicConfig(
+    level=logging.INFO,
+    format="%(asctime)s - %(name)s - %(levelname)s - %(message)s",
+)
+logger = logging.getLogger(__name__)
+
 # ─── FastAPI Application ───────────────────────────────────────────────────────
 
 app = FastAPI(
@@ -64,6 +76,38 @@ async def lifespan(app: FastAPI):
 # Add Security Headers Middleware
 app.add_middleware(SecurityHeadersMiddleware)
 
+
+# ─── Global Exception Handler ─────────────────────────────────────────────────
+
+
+@app.exception_handler(Exception)
+async def global_exception_handler(request: Request, exc: Exception):
+    """
+    Global exception handler to prevent sensitive data leakage.
+    Logs the full error server-side and returns a generic 500 response.
+    """
+    # Allow FastAPI's built-in exceptions to be handled normally
+    if isinstance(exc, StarletteHTTPException):
+        return JSONResponse(
+            status_code=exc.status_code,
+            content={"detail": exc.detail},
+        )
+    if isinstance(exc, RequestValidationError):
+        return JSONResponse(
+            status_code=422,
+            content={"detail": exc.errors()},
+        )
+
+    # Log the full exception for debugging
+    logger.error(f"Unhandled exception: {str(exc)}", exc_info=True)
+
+    # Return a generic error message to the client
+    return JSONResponse(
+        status_code=500,
+        content={"detail": "Internal server error"},
+    )
+
+
 # ─── Routers ──────────────────────────────────────────────────────────────────
 
 # Core Routers

tests/test_error_leakage.py

@@ -0,0 +1,63 @@
+import pytest
+from httpx import ASGITransport, AsyncClient
+from unittest.mock import patch
+from backend.main import app
+from backend.db.database import Base, engine, init_db
+
+@pytest.fixture(scope="module", autouse=True)
+async def setup_db():
+    async with engine.begin() as conn:
+        await conn.run_sync(Base.metadata.drop_all)
+    await init_db()
+    yield
+
+@pytest.mark.anyio
+async def test_error_leakage_mistral_gap_analysis():
+    """
+    Verify that the Mistral gap-analysis endpoint does not leak exception details.
+    """
+    # Request data
+    payload = {
+        "control_id": "AC.1.001",
+        "control_title": "Limit system access to authorized users",
+        "control_description": "Limit information system access to authorized users...",
+        "zt_pillar": "User",
+        "current_status": "not_implemented",
+        "existing_evidence": []
+    }
+
+    # Mock the agent's analyze_gap method to raise a sensitive exception
+    with patch("agents.mistral_agent.agent.agent.analyze_gap") as mock_analyze:
+        mock_analyze.side_effect = Exception("Sensitive database connection string: postgresql://admin:secret_password@db.internal:5432/cmmc")
+
+        async with AsyncClient(
+            transport=ASGITransport(app=app, raise_app_exceptions=False),
+            base_url="http://test"
+        ) as ac:
+            response = await ac.post("/api/agents/mistral/gap-analysis", json=payload)
+
+    assert response.status_code == 500
+    # Currently it leaks, so we expect this to FAIL once we start fixing it if we assert it DOES NOT contain it
+    # But for now, let's just assert it exists to confirm we have a working test that detects the leak.
+    # Actually, Sentinel's job is to fix it, so the test should assert the SAFE state.
+
+    detail = response.json().get("detail", "")
+    print(f"\nResponse detail: {detail}")
+    assert "secret_password" not in detail
+    assert "Sensitive database connection string" not in detail
+    assert detail == "Internal server error"
+
+@pytest.mark.anyio
+async def test_http_exception_not_swallowed():
+    """
+    Verify that standard HTTPExceptions (like 404) are still returned correctly.
+    """
+    async with AsyncClient(
+        transport=ASGITransport(app=app, raise_app_exceptions=False),
+        base_url="http://test"
+    ) as ac:
+        # Non-existent evidence ID
+        response = await ac.get("/api/evidence/non-existent-id-123")
+
+    assert response.status_code == 404
+    assert response.json()["detail"] == "Evidence non-existent-id-123 not found"