diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f07ab16..2794920 100644 --- a/.jules/sentinel.md +++ b/.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. diff --git a/agents/mistral_agent/agent.py b/agents/mistral_agent/agent.py index e09878d..cb92608 100644 --- a/agents/mistral_agent/agent.py +++ b/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} diff --git a/backend/main.py b/backend/main.py index 7c2d83b..bc93ac3 100644 --- a/backend/main.py +++ b/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 diff --git a/tests/test_error_leakage.py b/tests/test_error_leakage.py new file mode 100644 index 0000000..f590485 --- /dev/null +++ b/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"